v0.9.1: engineering-quality, architecture, and infrastructure follow-ups - #51
Merged
Conversation
…discarding them Fixes #43. Both bydensity FILTER() call sites now thread isDissimilar/isVISA into model.prelim/model.sifted alongside the existing unif output, rather than discarding them via ~. Not consumed by the pipeline itself, kept for later diagnostic inspection.
…ale subsetting Fixes #41. Adds opts.pilot.seed/opts.sifted.seed (defaulting from opts.general.seed, mirroring the existing opts.pythia.seed pattern), and replaces all five rng('default') call sites with a seeded rng('twister') using the appropriate seed. Previously these ignored opts.general.seed entirely, so replication/variance studies with different configured seeds got identical PILOT/SIFTED results. Adds a regression test confirming different seeds change PILOT's BFGS multi-start result, and that the same seed still reproduces bit-identically.
Fixes #36. SECURITY.md documents a private reporting channel (MATILDA contact page / GitHub private advisories) instead of the public issue tracker, plus a short scope note. CONTRIBUTING.md points to README's existing setup instructions rather than duplicating them, describes the example.m/test_integration.m pre-PR check, and records the repo's existing code-style conventions (header/licence block, ISA: error identifiers, centralised opts validation, comment-for-why-not-what). Both linked from README's Contact section.
…erparameter deferred item Fixes #33. Records, in RELEASE_NOTES.md's PYTHIA entry and CLAUDE.md's deferred-items section, that the refactor plan's "verify fitcensemble OptimizeHyperparameters support" item was resolved by architecture choice (one shared Sobol/Bayesian tuning layer for every classifier, ensemble included) rather than by the specific toolbox-compatibility check the plan named -- so it doesn't get mistakenly re-opened as still pending.
…t normalized Y Fixes #42. retrainLibsvmPythia was the only PYTHIA call site passing model.data.Y (normalized) instead of model.data.Yraw, unlike both production call sites in InstanceSpace.m. Avg_Perf_all_instances/Std_Perf_all_instances in the resulting summary are pure functions of the Y passed in, so this produced a visibly wrong (normalized-scale) summary table for any model migrated through this path, not just wrong training data internally. Adds a regression test comparing a retrained model's summary against a freshly-built model's summary on the same data.
…n-centred data Fixes #44. CLOISTER's sign(Xedge(i,j)) ~=/== sign(Xedge(i,k)) contradiction check only means anything for mean-centred data. PRELIM only mean-centres when both opts.auto.preproc and opts.norm.flag are true; with either off, a naturally all-positive feature (counts, sizes) makes the sign check degenerate for it with no indication anything went wrong. runCloister now warns (ISA:InstanceSpace:cloisterNotMeanCentred) when either flag is false, checked at the call site rather than inside CLOISTER.m itself so the standalone function's signature/API is unchanged. Adds a regression test confirming the warning fires with opts.norm.flag=false and does not fire on the default (normalised) path.
Fixes #29. svmpredict.mexw64/svmtrain.mexw64 were precompiled binaries with no corresponding source in the tree. Removed entirely (option (b) from the issue) rather than kept-with-provenance, since LIBSVM is already fully deprecated for new runs and ISAmigrateModel prefers retraining from scratch whenever the original training data is available. PYTHIA's eval mode now raises a clear ISA:PYTHIA:noLibsvm error, naming the algorithm and pointing to the official LIBSVM project (https://www.csie.ntu.edu.tw/~cjlin/libsvm/), instead of MATLAB's generic undefined-function error, if it ever needs to dispatch to a legacy LIBSVM-format classifier struct with svmpredict unavailable. ISAmigrateModel's cannotRetrainPythia warning updated to match. Updated README/CLAUDE.md/ liveDemoIS.m/the matlab-toolkit skill reference, which all previously implied the MEX-files were bundled in-repo. Adds a regression test confirming the new error fires for a legacy LIBSVM struct in eval mode.
Fixes #34. .github/workflows/tests.yml runs on every push (all branches) and on pull requests targeting master, via matlab-actions/setup-matlab (release: latest) + matlab-actions/run-command. Public-repo licensing covers all four required toolboxes (Global Optimization, Optimization, Parallel Computing, Statistics and Machine Learning) automatically, no secrets needed. example.m and test_integration.m both already abort with a nonzero exit on any uncaught error (test_integration.m explicitly raises ISA:test_integration:caseFailures on any case failure), so run-command's exit-code-based failure detection needs no extra pass/fail parsing. test/data/ outputs are uploaded as a workflow artifact on failure only, to help debug without bloating storage on every green run. Adds a Tests badge to README.
The first CI run (#34) failed immediately in PRELIM: "Unrecognized function or variable 'boxcox'. boxcox requires Financial Toolbox." README/liveDemoIS.m previously claimed the Financial Toolbox was NOT required -- wrong, and now corrected with a live MATLAB R2026a run as evidence rather than assumption. This also corrects part of the earlier #45 investigation, which guessed boxcox() resolved to the Statistics and Machine Learning Toolbox.
…best counts match Fixes #28. checkPrereq only confirmed the prerequisite STAGE completed, not that the specific obj.model fields that stage dereferences are actually present. Adds StageRequiredFields (a dotted-path field list per stage, deliberately not a full dependency-injection system) and checkRequiredFields, checked before dispatch in build()'s stage loop, so a missing field on a hand-edited model.mat, an incomplete legacy migration, or a model assembled outside the normal build() flow raises a clear ISA:InstanceSpace:missingField naming the stage and field, instead of an opaque crash deep inside PRELIM/SIFTED/PILOT/etc. No change to any successful-path behaviour. Also resolves the finding folded into this issue: TRACE.m's eval mode used ngood/nbest as the real-vs-placeholder boundary for out.good/out.best without ever checking they agree -- an implicit, never-asserted cross-file convention (relies on InstanceSpace.m's evaluateTestSet reconciliation and TRACE's own training-mode sizing staying in sync). Now asserts ngood==nbest before use, raising ISA:TRACE:goodBestCountMismatch on a real mismatch instead of silently producing misaligned footprints. Adds regression tests for both: a corrupted/incomplete model triggering checkRequiredFields, and a desynced trainedTrace.good/.best triggering the new TRACE assertion.
…dual mode Fixes #38, supersedes and closes #25/#37 (their scoped fixes are subsumed here, not done separately). Data ingestion previously had two independent, drifted implementations: the CSV-read + opts.selvars filtering embedded in InstanceSpace.runPrelim (build time), and a separate inline reimplementation in InstanceSpace.evaluateTestSet (explore time). core/INIT.m is a new standalone top-level function following the PYTHIA/TRACE nargin convention -- INIT(rootdir, opts) trains (reads metadata.csv), INIT(rootdir, opts, trainedModel) evaluates (reads metadata_test.csv, validates feature set/order, reconciles algorithm columns). Both runPrelim and evaluateTestSet now call INIT instead of maintaining their own copy. This half of the refactor is a pure extraction: each mode's behaviour is preserved exactly as it was, just no longer duplicated in two places that could silently drift apart. Design decision (the open question from #38's own description): PRELIM.m itself gets the nargin train/eval dual mode, rather than folding its Ybin/Ybest/P/beta computation into INIT. That computation (including tie-breaking) doesn't depend on train-vs-eval at all -- it's a pure function of Y and opts.perf.* -- so it now runs completely unconditionally, shared automatically between both modes. Only the bound-clipping/Box-Cox+ Z-score normalisation actually differs (fit fresh vs apply trainedPrelim's already-fit parameters), so only that part is nargin-branched. This is what concretely closes #37: explore-time tie-breaking used to be a silent, deterministic sort() with no tie-breaking logic at all, independently drifted from PRELIM's own random tie-break for instances with more than one best-performing algorithm. Sharing one code path makes that drift structurally impossible going forward. evaluateTestSet's signature changed from (model, datafile) to (model, rootdir) so it can call INIT the same way runPrelim does; its sole caller (explore()) already had testRootDir available directly. Adds a regression test proving PRELIM's train and eval modes now break an identical synthetic tie identically when seeded the same way -- the direct evidence that both modes share code rather than merely producing similar output. The full existing test_integration.m suite already exercises the whole INIT+PRELIM eval path via exploreIS() on every case, so the extraction itself is covered by the existing broad suite, not just the new targeted test. CLAUDE.md updated: INIT added to the core/ file list, the train/eval convention note now includes PRELIM/INIT, and the #38 design decision is recorded so it isn't re-litigated later without new evidence.
Fixes #39. Replaces the hand-rolled script (per-case try/catch, manual pass/fail bookkeeping) with matlab.unittest.TestCase classes under tests/: - PipelineOptionsTest: the 20-case option-coverage cell array is now a TestParameter (OptionCase), run by one parameterized test method instead of a manual loop -- matches the issue's "ideally parameterised, not a 1:1 mechanical port" preference. - ClassApiTest: the staged build()/explore()/save-load/invalidation sequence, kept as one test method since each step depends on state left by the previous one. - MigrationTest: the full ISAmigrateModel legacy-migration table, split into independent test methods where the pre-#39 script's cases already were independent; a TestClassSetup builds one shared trained model for the LIBSVM-retraining/legacy-TRACE-recompute cases, replacing the old script's "reuse obj from class_api if available, else build one" workaround with a proper shared fixture. - RegressionTest: the #41/#44/#28/#37+#38 targeted regressions, each its own test method. - testDefaultOpts.m: the baseline opts struct, shared by all four classes (previously duplicated per test-class draft during this migration itself -- consolidated before commit rather than propagating four copies). test_integration.m is now a thin runner: TestSuite.fromFolder('tests') + TestRunner with CodeCoveragePlugin (Cobertura report, coverage.xml), preserving the EOF:SUCCESS/EOF:ERROR sentinel exactly (buildIS.m/ exploreIS.m use the same convention independently of this file, per the issue's own acceptance criteria). No changes needed to .github/workflows/tests.yml's invocation (still `command: example, test_integration`) -- only an added always-upload step for coverage.xml. example.m is untouched, per the issue's explicit scope boundary. CLAUDE.md's Test harness section updated to describe the new structure.
Run 32301539795 hung for ~2h45m inside setup-matlab's system-dependency apt-get step (a runner-side network/mirror stall, unrelated to any code change) before it had to be cancelled manually. Adds timeout-minutes so a repeat fails fast and visibly instead of silently running toward GitHub's 6-hour default job timeout.
Fixes a real bug in the #39 migration, confirmed directly from CI logs (run 32315024151): matlab.unittest does not run TestClassSetup/test methods with the working directory test_integration.m was launched from -- observed as './test/data/...' resolving to 'tests/test/data/...' inside every TestClassSetup, and PipelineOptionsTest's OptionCase TestParameter (which reads metadata.csv at class-parse time) failing outright and excluding the whole class from the suite. Adds tests/testRepoRoot.m (fileparts(fileparts(mfilename('fullpath'))), robust to caller cwd by construction) and uses it in all four test classes instead of the relative literal. test_integration.m itself is unaffected -- its own top-level code runs before TestSuite.fromFolder shifts the working directory, matching example.m's already-working relative-path usage in the same MATLAB session.
…test execution Fixes the second #39 bug confirmed by CI (run 32316118276), one layer under the path bug fixed in 6250a0f: buildIS.m/exploreIS.m/InstanceSpace.m live at the repo root and were never formally on the MATLAB search path -- they only resolved via MATLAB's implicit current-folder lookup, which broke the moment matlab.unittest shifted the working directory away from the repo root during test execution ("Undefined function 'buildIS'"/"'InstanceSpace' for input arguments of type 'char'/'struct'"). core/output/utils were unaffected since InstanceSpace.ensurePathSetup already puts them on the path explicitly, independent of cwd. test_integration.m now captures pwd (repo root, confirmed by example.m's already-working relative-path usage earlier in the same MATLAB session) and addpath()s it before the test suite runs, once, for the rest of the session.
Replaces the version-specific badge/DOI (10.5281/zenodo.4750845) with the repo-linked badge (zenodo.org/badge/144672744.svg) pointing at 10.5281/zenodo.4484107, matching the DOI already used in the citation text further down the file -- the two had pointed at different DOIs.
…ndary display Fixes #31 and #32 (option (a): 2D-only, deferring CLOISTER's own 2D-only convex hull to a separate follow-up -- see CLAUDE.md). #31: traceAlphaBoundary (moved from scriptcsv.m into scriptfcn.m, for testability alongside the other shared plotting helpers) used to build one adjacency graph from boundaryFacets(poly)'s combined edge list -- every disconnected region's edges flattened together -- and stop as soon as it ran out of same-loop neighbours, silently returning only the first region traced for a multi-region alpha shape. It now traces each boundaryFacets(poly, regionID) region independently and concatenates them, NaN-separated (the same multi-part-polyline convention polyshape.Vertices already uses). Single-region shapes (the common case) are unaffected: no separator rows, identical output to before. The Python-derived "retry with a re-optimised alpha" pattern the issue proposed turned out not to apply here: scriptfcn.m's plot() already renders multi-region alphaShapes correctly via MATLAB's native plotting, so the bug was specifically in this hand-rolled CSV vertex tracer, not in TRACE's alpha selection -- changing the alpha would have changed the exported footprint's actual geometry to work around an export-format limitation, and drifted from what the PNG output shows. #32: CLOISTER's boundary was computed (model.cloist.Zedge/Zecorr) but never rendered in automated output or exposed via plot(). Adds drawBoundary (scriptfcn.m: grey instance scatter + red boundary outline), a distribution_boundary.png in scriptpng.m (skipped when CLOISTER wasn't run, e.g. an evaluateTestSet result, or when opts.pilot.dims==3), and an InstanceSpace.plot('boundary') case mirroring the other views. liveDemoIS.m's manual boundary-plotting cell now just calls obj.plot('boundary'). Scoped to 2D only (option (a)): CLOISTER's own Zedge/Zecorr computation is a 2D-only convex hull regardless of projection dimensionality (core/CLOISTER.m's convhull(Z(:,1),Z(:,2))), a separate, deliberately out-of-scope bug recorded in CLAUDE.md rather than fixed here, since changing what CLOISTER computes is an algorithmic change outside v0.9.1's stated scope. Both the plot() case and the PNG explicitly refuse a 3D projection (ISA:InstanceSpace:boundaryNot3D) rather than render an inaccurate boundary. Adds regression tests: a genuine two-cluster multi-region alphaShape confirming traceAlphaBoundary now returns both regions (#31), and 2D boundary-display + 3D-rejection coverage (#32).
Zenodo's GitHub integration failed to archive the v0.9.0 release
("Citation metadata load failed"): Zenodo validates CITATION.cff's license
field strictly against the official SPDX License List, and
LicenseRef-PolyForm-Noncommercial-1.0.0 is not on it (PolyForm
Noncommercial 1.0.0 is not an OSI-approved/SPDX-registered license). The
CFF spec's own documented mechanism for a non-SPDX license is license-url
instead of license, pointing at the licence text -- matching what every
other file header in this repo already cites.
Note for the next release: this only fixes future archival attempts: the
v0.9.0 release already failed to archive on Zenodo and will need a new
release (or Zenodo's "Sync now" re-request, if available) to pick this up.
…, #27) build('onStage', @(stageName, model) ...) fires after each stage completes, letting callers inspect intermediate results (e.g. PILOT's projection) without manually breaking a run into several staged calls. explore(testRootDir, 'onStage', @(stageName, out) ...) does the same for the five conceptual stages evaluateTestSet runs internally (prelim, sifted, pilot, pythia, trace -- cloister is never recomputed at explore time). Both are purely additive: omitting the callback changes nothing.
#30 (PILOT orientation canonicalisation) proposes a new algorithmic step and opts toggle, out of v0.9.1's engineering-only scope -- deferred, comment left on the issue. #35 (Python-port fixture export tool) closed as not planned: cross-repo Python-port work belongs in that repo, not here. #45 (verify PRELIM's boxcox() call) closed as resolved -- confirmed it resolves to the Financial Toolbox's boxcox.m, the same dependency CI already installs for this call site, with no gap found alongside the existing positive-shift guard.
Adds a new v0.9.1 section to RELEASE_NOTES.md (new functionality, better engineering, bug fixes) covering everything since the v0.9.0 tag, without touching the existing v0.9.0 section. Bumps Contents.m/CITATION.cff to 0.9.1. Updates README.md: onStage callback example, opts.pilot.seed/ opts.sifted.seed, opts.perf.epsilon's AbsPerf-dependent range, CLOISTER's boundary-display output and non-mean-centred-data warning, and corrects the test_integration.m/options.json section to describe the current matlab.unittest-based tests/*.m structure instead of the pre-#39 script. Same set of corrections applied to the Claude Code skill's matlab-toolkit.md reference, plus its verified-against version bump.
Root cause of the CI failure on 905acdd: the test referenced InstanceSpace.StageOrder directly from outside the class, which is Access = private -- an access-denied runtime error (matlab.unittest reports this as "Errored", not "Failed"), not a genuine test-coverage gap. build()/explore() themselves are unaffected; only the test needed its own hardcoded expected stage order, matching how the explore() half of the same test already did it.
The previous version only exercised onStage's "run everything, inspect along the way" mode (build() with no 'stages' filter), which is why it needed to hardcode the full canonical stage list. It never exercised onStage combined with build()'s existing 'stages' subset flexibility -- the arguably more useful combination, and the one #26's own docstring example implies. Added a case that requests {'prelim','sifted','pilot'} with onStage and asserts the callback fires only for that subset, in that order -- no hardcoded full list needed, since the expectation is just "echo back what was requested".
Contributor
There was a problem hiding this comment.
Pull request overview
This PR lands the v0.9.1 milestone work on master, focusing on engineering-quality and infrastructure follow-ups to v0.9.0 (with explicit scope constraints: no algorithmic changes and no new user-facing options). It adds CI, modernizes the MATLAB test harness, consolidates duplicated ingestion logic into a shared train/eval entry point, and includes several targeted correctness and ergonomics improvements across the pipeline.
Changes:
- Adds GitHub Actions CI and migrates the regression suite to
matlab.unittestwith code coverage output (coverage.xml). - Introduces shared train/eval ingestion (
core/INIT.m) and extendsPRELIMto anargin-dispatched eval mode; adds per-stageonStagecallbacks and required-field contract checks. - Fixes several confirmed regressions/edge cases (seed threading, LIBSVM migration retraining data scale, TRACE eval invariant checks) and updates documentation/release metadata for v0.9.1.
Reviewed changes
Copilot reviewed 31 out of 33 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/ISAvalidateOpts.m | Validates newly introduced pilot.seed / sifted.seed options. |
| utils/ISAmigrateModel.m | Fixes LIBSVM-era retraining path to use Yraw; improves warning guidance. |
| utils/ISAdefaults.m | Adds defaults for opts.pilot.seed and opts.sifted.seed. |
| core/INIT.m | New shared ingestion function for build/explore paths (train/eval by nargin). |
| core/PRELIM.m | Adds eval mode to apply trained preprocessing while sharing tie-breaking logic. |
| InstanceSpace.m | Adds onStage callbacks, field-level contract checks, boundary plotting, INIT-based ingestion, and seed threading for small-scale subsetting. |
| core/PILOT.m | Uses opts.seed instead of rng('default') for BFGS restarts. |
| core/PILOTviewpoint.m | Uses opts.seed for viewpoint optimization restarts. |
| core/SIFTED.m | Uses opts.seed instead of rng('default') for k-means/cluster steps. |
| core/TRACE.m | Adds explicit eval-mode invariant guard for good/best count mismatch. |
| core/PYTHIA.m | Adds explicit error when legacy LIBSVM struct is present but svmpredict is unavailable. |
| output/scriptpng.m | Adds automated CLOISTER boundary PNG output for 2D projections. |
| output/scriptfcn.m | Centralizes boundary drawing and alpha-shape boundary tracing utilities (multi-region fix). |
| output/scriptcsv.m | Uses centralized boundary extraction; documents NaN-separated multi-region boundaries. |
| liveDemoIS.m | Switches manual boundary plotting to obj.plot('boundary'); updates toolbox requirements text. |
| test_integration.m | Becomes a matlab.unittest runner wiring in Cobertura coverage and preserving EOF sentinels. |
| tests/testRepoRoot.m | Adds robust repo-root discovery for tests independent of cwd. |
| tests/testDefaultOpts.m | Shared baseline opts for test suite. |
| tests/PipelineOptionsTest.m | Parameterized option-surface coverage via matlab.unittest. |
| tests/ClassApiTest.m | Class API staged build/save/load/explore coverage, plus onStage callback tests. |
| tests/MigrationTest.m | Migration-table coverage + LIBSVM/Yraw regressions in matlab.unittest. |
| tests/RegressionTest.m | Targeted regression tests for fixed issues (#28/#31/#32/#37/#38/#41/#44 etc.). |
| .github/workflows/tests.yml | Adds CI workflow running example.m + test_integration.m and uploading artifacts. |
| README.md | Adds CI badge; updates dependencies (Financial toolbox for boxcox), docs for tests and options, boundary, and LIBSVM status. |
| RELEASE_NOTES.md | Adds v0.9.1 release notes section and preserves existing v0.9.0 notes. |
| SECURITY.md | Adds security policy document. |
| CONTRIBUTING.md | Adds contributor guide. |
| Contents.m | Bumps version/date to 0.9.1. |
| CITATION.cff | Bumps version/date; switches to license-url for non-SPDX license. |
| CLAUDE.md | Updates repository operational notes consistent with v0.9.1 changes. |
| .claude/skills/instance-space-analysis/references/matlab-toolkit.md | Updates skill reference doc for v0.9.1 defaults and behaviors. |
Suppressed comments (1)
SECURITY.md:41
- This bullet says the LIBSVM MEX binaries are bundled, but the PR removes them. Update this to avoid implying the repo ships binary artifacts.
- The bundled LIBSVM MEX binaries, used only for migrating pre-v1.7
legacy models (see #29 for their provenance status).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- CONTRIBUTING.md: CI now exists (#34), so drop the "no CI yet" note; PRELIM/INIT also implement the train/eval dual-mode convention now (#38), not just PYTHIA/TRACE. - SECURITY.md: LIBSVM's MEX files are no longer bundled (#29, removed), not merely a bundled binary to be wary of -- reworded both mentions. - core/INIT.m: revert featureCountMismatch/featureOrderMismatch back to their pre-#38 ISA:InstanceSpace:* identifiers. They'd drifted to ISA:INIT:* when this validation moved out of InstanceSpace.m into the new INIT.m, silently breaking any caller matching on the old identifier (this validation is reachable through explore(), a public entry point MATILDA's web platform calls). - test_integration.m: derive repoRoot from mfilename('fullpath') instead of pwd, so it doesn't add the wrong directory to the path if launched from somewhere other than the repo root.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 33 changed files in this pull request and generated no new comments.
Suppressed comments (16)
core/SIFTED.m:163
- The configured SIFTED seed still does not control the CV partition or genetic algorithm: line 165 restores the caller RNG before both are created. This also leaves standalone
SIFTED(...)calls without a locally defaultedseedfailing on line 145. Keep the seeded state through GA completion with anonCleanupguard and provide the same standalone default used by PYTHIA.
rng(opts.seed, 'twister');
out.clust = bsxfun(@eq, kmeans(Xaux', opts.K, 'Distance', 'correlation', ...
'MaxIter', opts.MaxIter, ...
'Replicates', opts.Replicates, ...
'Options', statset('UseParallel', nworkers~=0), ...
README.md:190
- The migration path now explicitly requires
model.data.Yraw, but this updated README still tells users thatmodel.data.Yis sufficient. A model with only normalizedYwill take the warning/skip path, so document the actual required field.
The toolkit selects one binary classifier per algorithm (good/not-good performance) from a registry of MATLAB-native classifiers, resolved via `ISAgetClassifierFcn`. It runs after PILOT, on the projected instance space, and does not depend on CLOISTER. **LIBSVM is deprecated for new runs**: `buildIS` never dispatches to it. `ISAmigrateModel` (see below) renames legacy field names on an old model (e.g. `.svm`/`.knn` → `.classifiers`), and additionally **retrains** any classifiers still in the legacy LIBSVM struct format (no `predict()` method, so they can't just be relabelled) using the current registry — `opts.pythia.classifier` if already set to a valid registry name, `'knn'` otherwise — provided the model still has `model.pilot.Z` and `model.data.Y`/`Ybin`/`Ybest`/`algolabels` to retrain from. If those fields are missing, migration proceeds but leaves the legacy classifiers in place with a warning; such a model can still be evaluated through `exploreIS`/`PYTHIA` eval mode (which dispatches to `svmpredict` when it detects a struct instead of a MATLAB classifier object), but this requires the LIBSVM MEX-files, which are **not bundled with this repository** (see the Installation Instructions section above) — obtain them from [the official LIBSVM project](https://www.csie.ntu.edu.tw/~cjlin/libsvm/) if you actually need to go down this path. Prefer retraining from scratch with `buildIS`, or a migration with the full training data available, to drop that dependency entirely.
.claude/skills/instance-space-analysis/references/matlab-toolkit.md:154
- The operational reference uses
prelim.norm, but users configure normalization throughopts.norm.flag;prelim.normis only an internal flattened field passed to PRELIM. Update this public-facing option name.
`prelim.norm=false`, a warning (`ISA:InstanceSpace:cloisterNotMeanCentred`,
core/PILOT.m:206
- This makes
seedmandatory for directPILOT(...)callers, even though this standalone API previously worked with the documented stage options and locally defaults other newly optional fields.ISAdefaultsonly protects calls routed throughInstanceSpace; add a local seed fallback so existing standalone numerical calls do not fail with “Unrecognized field name 'seed'.”
rng(opts.seed, 'twister');
core/PRELIM.m:207
- This error used to be raised by public
InstanceSpace.explore()asISA:InstanceSpace:complexX. Moving the check into PRELIM and changing its identifier breaks callers that catch the established identifier, just like the feature-count/order identifiers already preserved in INIT. Keep the original identifier for this eval-only path.
if ~isreal(X)
error('ISA:PRELIM:complexX', ...
'Feature matrix X is complex after normalisation. Check test data range vs training data.');
test_integration.m:75
- The runner is still cwd-dependent despite deriving
repoRoot: suite discovery, test-data creation, coverage source folders, and the report path remain relative. Calling this script from another directory therefore searches that caller’stests/(or fails) instead of this repository. Build all these paths fromrepoRoot.
suite = TestSuite.fromFolder('tests', 'IncludingSubfolders', true);
core/PILOTviewpoint.m:124
- Direct
PILOTviewpoint(...)calls are now required to supply a newopts.seedfield, while this function locally defaults its other optional fields and does not runISAdefaults. Preserve the standalone API by defaulting the seed locally when absent.
rng(opts.seed, 'twister');
core/PRELIM.m:163
- Moving explore-time clipping into PRELIM dropped the existing
ISA:InstanceSpace:outOfDistributionwarning that fired when more than 5% of test instances were clipped. That is a user-visible regression inexplore()and removes the only indication that test data may lie outside the training distribution; retain the warning before applying the masks.
himask = bsxfun(@gt, X, trainedPrelim.hibound);
lomask = bsxfun(@lt, X, trainedPrelim.lobound);
X = X.*~(himask | lomask) + bsxfun(@times, himask, trainedPrelim.hibound) + ...
bsxfun(@times, lomask, trainedPrelim.lobound);
README.md:186
opts.prelim.normis not a public option in this repository; the implemented warning checksopts.norm.flag. As written, users following this new guidance will set an ignored field and receive no warning. Use the actual option name.
This issue also appears on line 190 of the same file.
CLOISTER's correlation-contradiction filter assumes mean-centred feature data (its sign-based check is only meaningful when values span both positive and negative territory); if ```opts.prelim.norm = false```, a warning (```ISA:InstanceSpace:cloisterNotMeanCentred```) is raised, since a naturally all-positive feature would otherwise silently make the check degenerate for it.
RELEASE_NOTES.md:44
- This release note names
opts.prelim.norm, but the supported full options structure and the new guard useopts.norm.flag. Correcting the field name is necessary for the documented reproduction to work.
- **CLOISTER's correlation-contradiction filter silently degraded under non-mean-centred data.** Its sign-based contradiction check (`sign(Xedge(i,j)) ~=/== sign(Xedge(i,k))`) is only meaningful when feature values span both positive and negative territory — true by default (CLOISTER receives Box-Cox+Z-scored data), but not when `opts.prelim.norm=false`, a legitimate documented option. A naturally all-positive feature (counts, sizes) under that setting made the check degenerate with no indication anything was off. `InstanceSpace.m` now warns (`ISA:InstanceSpace:cloisterNotMeanCentred`) when this precondition is violated.
.claude/skills/instance-space-analysis/references/matlab-toolkit.md:6
- The v0.9.1 documentation now correctly declares Financial Toolbox as required for
boxcox(), but this operational reference’s updated prerequisite sentence still omits it. Add Financial Toolbox so the reference does not send users to an incomplete environment setup.
This issue also appears on line 154 of the same file.
v0.9.1). Requires **MATLAB R2025a or later**, with the Global
Optimization, Parallel Computing, Optimization, and Statistics and
Machine Learning toolboxes. Re-verify against the live repo if a detail
.github/workflows/tests.yml:23
- CI currently tests
latest, but the release and README promise support from MATLAB R2025a onward. Oncelatestadvances, a green build no longer validates the minimum supported release and can also drift unexpectedly. Pin this job to R2025a (or add an R2025a matrix entry).
release: latest
tests/MigrationTest.m:149
- This regression test fails in a valid documented environment where a contributor has installed official LIBSVM: PYTHIA then calls
svmpredictinstead of raisingISA:PYTHIA:noLibsvm. Skip the negative-path assertion when the optional MEX-file is present so installing the supported external dependency does not break the suite.
testCase.verifyError(@() PYTHIA(baseModel.pilot.Z, baseModel.data.Yraw, baseModel.data.Ybin, ...
RELEASE_NOTES.md:3
- The release claims there are no new user-facing options, but this PR adds, validates, and documents
opts.pilot.seedandopts.sifted.seedas independently configurable options. Reword the scope statement so it does not contradict the shipped API.
This release is an engineering-quality, architecture, and infrastructure follow-up to v0.9.0 — deliberately scoped to **no algorithmic changes and no new user-facing options** (see the `v0.9.1` GitHub milestone). What did land: two small, additive API surfaces (a per-stage inspection callback, and finally rendering a boundary CLOISTER had computed all along), a real architectural clean-up of data ingestion, and a batch of confirmed correctness fixes found via a full-repository audit against the project's own established conventions. It targets **MATLAB R2025a or later**.
InstanceSpace.m:369
- This message points users to closed issue #32, while the unresolved 3D boundary work is explicitly tracked as #50 throughout this PR. Point to #50 so the actionable follow-up is discoverable.
error('ISA:InstanceSpace:boundaryNot3D', ...
['The ''boundary'' view is 2D only: CLOISTER''s empirical bound is not ' ...
'yet computed for 3D projections (opts.pilot.dims==3). See issue #32.']);
RELEASE_NOTES.md:49
- The previous v0.9.0 heading was replaced by the new v0.9.1 heading, but no heading was inserted before the retained v0.9.0 content below. As a result, the historical v0.9.0 changelog now appears to be part of v0.9.1. Restore its section heading after this separator.
---
…dings)
Fixed critical error first: my earlier RELEASE_NOTES.md edit had deleted
the "# ... v0.9.0" heading itself (used as the old_string anchor), making
the historical v0.9.0 changelog read as part of v0.9.1 -- restored.
Real correctness fixes:
- SIFTED.m: the seeded RNG state was restored right after kmeans, before
cvpartition/ga() ran -- both silently ran under the caller's original
RNG instead of opts.seed. Now held through GA completion via an
onCleanup guard (PYTHIA's established pattern), not a manual restore.
- PILOT.m/SIFTED.m/PILOTviewpoint.m: opts.seed is now unconditionally
read, but only ISAdefaults (i.e. calls routed through InstanceSpace)
provides it -- a standalone call, which CLAUDE.md requires stay
supported, would error with "Unrecognized field name 'seed'". Added
the same local `if ~isfield(opts,'seed') opts.seed = 42; end` default
PYTHIA already uses.
- PRELIM.m: restored two identifiers/behaviour dropped when this logic
moved out of InstanceSpace.m -- ISA:PRELIM:complexX reverted to
ISA:InstanceSpace:complexX (same backward-compat reasoning as the
INIT.m identifiers fixed in the previous review round), and the
>5%-clipped ISA:InstanceSpace:outOfDistribution warning restored to
PRELIM's eval-mode bound-clipping (silently dropped, not carried over).
- test_integration.m: mkdir/TestSuite.fromFolder/coverage source
folders/report path were still pwd-relative even after the previous
round's repoRoot fix; now all built from repoRoot.
- .github/workflows/tests.yml: pinned release: R2025a instead of
'latest', so CI keeps validating the documented minimum instead of
silently drifting as new MATLAB versions ship.
- tests/MigrationTest.m: guarded testNoLibsvmRegression with
assumeTrue(exist('svmpredict','file')==0) -- a contributor with LIBSVM
actually installed would otherwise get a spurious failure.
Doc accuracy fixes: opts.prelim.norm corrected to the real
opts.auto.preproc/opts.norm.flag fields (README.md, RELEASE_NOTES.md,
matlab-toolkit.md -- opts.prelim.norm isn't a real field); README's
LIBSVM-migration paragraph corrected from model.data.Y to the actually-
required model.data.Yraw; matlab-toolkit.md's prerequisite sentence was
missing the Financial Toolbox; RELEASE_NOTES.md's "no new user-facing
options" claim softened to acknowledge opts.pilot.seed/opts.sifted.seed
exist (as a side effect of making the already-documented
opts.general.seed behave as promised, not a new capability); the
boundaryNot3D error pointed at closed issue #32 instead of the actual
open follow-up, #50.
Owner
Author
|
Addressed the second Copilot review round (16 suppressed-confidence findings) in a3aa2d4. All 16 checked directly against source before acting — full breakdown in the commit message. Highlights:
CI is running on a3aa2d4 now. Generated by Claude Code |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 33 changed files in this pull request and generated 1 comment.
Suppressed comments (6)
InstanceSpace.m:832
- PRELIM's shared tie-breaking now consumes
randi, butexplore()never seeds or restores the RNG. Consequently, evaluating the same model and test data twice can choose differentPvalues—and therefore different TRACEbestrescoring—based on unrelated prior RNG use. Seed this call from the frozen model seed while restoring the caller's RNG state.
[out.data.X, out.data.Y, prelimOut] = PRELIM(out.data.X, out.data.Y, prelimOpts, model.prelim);
output/scriptfcn.m:501
- A single alpha-shape region can contain multiple boundary cycles (for example, an outer ring plus a hole). In that case
boundaryFacets(poly, r)returns disconnected edge loops, but this helper follows only the loop containingbf(1,1)(and then repeats it), silently omitting hole boundaries from the CSV. Trace every unvisited edge cycle within each region and NaN-separate those cycles too.
% correctly for a simple, single connected boundary -- traceAlphaBoundary
% calls this once per region rather than once for a whole (possibly
% multi-region) shape, which is what makes that assumption safe again.
.claude/skills/instance-space-analysis/references/matlab-toolkit.md:278
- This operational reference says both paths raise
boundaryNot3D, butscriptpng.msimply skips its boundary block whenis3D; onlyInstanceSpace.plot('boundary')raises that error. Document the silent PNG omission separately so users do not expect a 3D build to fail.
other views. Both explicitly raise `ISA:InstanceSpace:boundaryNot3D`
tests/PipelineOptionsTest.m:17
- The generated options are based on
testDefaultOpts(), not abaseOpts()function in this file. This instruction currently sends contributors to a nonexistent helper.
% next run overwrites it. To change what gets tested, edit baseOpts()
% (this file) or the relevant OptionCase entry.
InstanceSpace.m:111
- The SIFTED stage unconditionally dereferences
obj.model.featsel.idxwhen updating the selected feature mapping, but this contract omits it. A reconstructed model can pass the new validation and still fail opaquely insiderunSifted, which defeats the purpose of the field-level guard.
This issue also appears on line 832 of the same file.
'sifted', {{'data.X', 'data.Y', 'data.Ybin', 'data.featlabels'}}, ...
RELEASE_NOTES.md:19
- Only
plot('boundary')raisesISA:InstanceSpace:boundaryNot3D;scriptpng.msilently skipsdistribution_boundary.pngfor a 3D model. The release note should distinguish these behaviors rather than claiming both emit the error.
**CLOISTER boundary is now actually rendered, for 2D projections.** CLOISTER has always computed an empirical bound (`model.cloist.Zedge`/`Zecorr`) for the instance space, but nothing in the automated pipeline ever drew it — a three-year-old deferred item from the original refactor plan's design review that Phase 9 was supposed to close and didn't. Now: `scriptpng.m` writes a `distribution_boundary.png`, and `InstanceSpace.plot('boundary')` works like the other views. Both explicitly refuse a 3D projection (`ISA:InstanceSpace:boundaryNot3D`) rather than render a boundary that would silently be wrong in the third dimension — CLOISTER's own hull computation is still 2D-only regardless of projection dimensionality, tracked separately as a genuine algorithmic follow-up ([#50](https://github.com/andremun/InstanceSpace/issues/50), out of this release's scope).
Real correctness fixes: - InstanceSpace.m: runPrelim pruned data.Yraw/Y/Ybin/algolabels when an algorithm has no good instances, but left prelimOut.lambdaY/muY/sigmaY (per-algorithm, fit before pruning) untouched. PRELIM's eval mode derives modelalgos from numel(trainedPrelim.lambdaY), so this both over-counted modelalgos against the actually-reconciled eval-time Y (indexing past it, or misapplying a pruned algorithm's transform to an unrelated new algorithm's column) and, for any pruned algorithm that wasn't last, silently misaligned every surviving lambda/mu/sigma positioned after it. Now pruned with the same mask as data.algolabels. Regression test added directly against PRELIM (forcing real pruning through the full pipeline needs data engineered so one algorithm is never "good," which is fragile against the bundled reference dataset; simulating the post-prune trainedPrelim state is equivalent and deterministic), pruning the middle algorithm specifically since that's what exposes the positional-misalignment half of the bug. - InstanceSpace.m: explore() shares PRELIM's tie-breaking code with build() (per #37/#38), including its randi() draw for ties, but never seeded the RNG before calling PRELIM -- two explore() calls on the identical model and test data could pick different tied algorithms depending on unrelated prior RNG use. Seeded from the frozen model's own opts.general.seed, restored via onCleanup (PYTHIA/SIFTED's existing pattern). - InstanceSpace.m: StageRequiredFields for 'sifted' omitted featsel.idx, which runSifted() unconditionally read-then-writes (obj.model.featsel.idx = obj.model.featsel.idx(...)) -- a genuine prerequisite the #28 contract-validation work had missed. A model missing it crashed opaquely inside runSifted instead of via the intended clear error. Added to the contract; regression test extended. - output/scriptfcn.m: traceOneRegion assumes a region's boundary is one closed cycle, but a region with a hole (numRegions counts connected point-set components, not boundary cycles) has two disconnected cycles -- outer ring and hole -- and the tracer only ever reaches the one containing its arbitrary starting vertex. Rather than attempt full multi-cycle tracing here (a real algorithmic addition, out of v0.9.1 scope), it now warns (ISA:scriptfcn:boundaryHoleOmitted) instead of silently dropping the hole. Filed as #52 for the actual fix, tracked in CLAUDE.md alongside #50. Doc accuracy fixes: RELEASE_NOTES.md/matlab-toolkit.md/CLAUDE.md corrected -- scriptpng.m silently skips distribution_boundary.png for a 3D projection (no error, no warning); only plot('boundary') raises ISA:InstanceSpace:boundaryNot3D. Previously described as if both "explicitly refuse" 3D. tests/PipelineOptionsTest.m's stale baseOpts() references (a function that doesn't exist in this file) corrected to testDefaultOpts(), the actual shared-settings function.
Owner
Author
|
Addressed the third review round's 6 suppressed-confidence findings in 28223c9, alongside the posted
CI is running on 28223c9 now. Generated by Claude Code |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 33 changed files in this pull request and generated no new comments.
Suppressed comments (1)
output/scriptfcn.m:525
- The hole safeguard does not trigger as written. After the first boundary cycle closes, this traversal can revisit its starting vertex and keep repeating that cycle until
orderis full, sovalidis all true even though the disconnected hole cycle was omitted. Exclude already visited neighbours so the loop breaks and the warning below can report the omitted vertices.
nxt = adj(curr, adj(curr,:) ~= prev & adj(curr,:) ~= 0);
Copilot review round 4 on #51. traceOneRegion's walk only stopped on isempty(nxt) (a dead end) -- it never checked for returning to the start vertex. For a region with a hole (two disconnected boundary cycles), excluding just `prev` still leaves a "next" vertex available once back at the start, so the walk didn't halt when the outer cycle closed; it just kept re-treading the same cycle's vertices until `order` filled up completely. That left `valid` all true and the ISA:scriptfcn:boundaryHoleOmitted warning added in the previous review round -- specifically for this case -- silently never firing, with duplicated outer-ring vertices in the output instead of the intended "outer ring only, flagged" result. Added a stop condition for nxt(1) == order(1) (back at the start). No-op for the ordinary single-cycle case: the for loop's own range (nRegionVerts) already ends before a closed cycle shorter than that could ever reach this check, so nothing changes there. Exposed traceOneRegion via scriptfcn's existing assignin mechanism (matching footprintBoundary/traceAlphaBoundary/drawBoundary) and added a regression test against a synthetic two-cycle edge list (a 4-vertex outer square plus a disconnected 3-vertex inner triangle) rather than a geometric alphaShape, for full control over the exact adjacency instead of depending on alphaShape's own region/facet algorithm to reliably reproduce a specific hole shape.
Owner
Author
|
Addressed the fourth review round's finding in 4c31755 — a real bug in my own previous fix, confirmed by tracing through the logic by hand:
Fixed by adding a Exposed Generated by Claude Code |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 33 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
output/scriptfcn.m:530
- This traversal only excludes the immediately previous vertex. After closing the first boundary cycle it revisits the starting vertex and keeps filling
order, so for a region with a holevalidcan remain all true and the new omission warning never fires. Exclude already visited vertices so the loop stops when that cycle closes and the remaining cycle is detected.
nxt = adj(curr, adj(curr,:) ~= prev & adj(curr,:) ~= 0);
output/scriptpng.m:234
- Skipping this export for a 3D/no-CLOISTER run leaves any
distribution_boundary.pngfrom an earlier 2D build in the same output directory. That stale image then appears to belong to the current build even though it depicts the old 2D model; remove the existing file when the boundary is not applicable.
if isfield(container, 'cloist') && ~is3D
clf;
drawBoundary(container.pilot.Z, container.cloist.Zedge, 'CLOISTER empirical bound');
exportgraphics(fig, [rootdir 'distribution_boundary.png']);
end
InstanceSpace.m:111
- The SIFTED contract omits its conditional
data_dense.X/Y/Ybininputs. Whenmodel.prelim.bydensityis true,runSifteddereferences those fields unconditionally, so an incomplete loaded/edited model still passes this check and fails with the opaque missing-field error this validation is intended to prevent. Add those requirements conditionally for density mode.
'sifted', {{'data.X', 'data.Y', 'data.Ybin', 'data.featlabels', 'featsel.idx'}}, ...
output/scriptfcn.m:542
- Use “vertex/vertices”; “vertice(s)” is not a valid singular or plural form.
['This region''s boundary has %d vertice(s) not reachable from its outer ring -- ' ...
tests/testRepoRoot.m:4
- The framework name is
matlab.unittest, nottest.unittest.
% because MATLAB's current working directory during actual test.unittest
Real correctness fixes: - core/TRACE.m: the #28 ngood==nbest guard only proved good/best agree with EACH OTHER, not that either still holds the true trained algorithm count -- if both were shortened by the same amount (e.g. the same buggy migration/edit), the check passed and the "lost" algorithms were silently replaced with empty placeholders instead of erroring. Cross-checked against trainedTrace.summary (cell(nalgos+1, 11), unconditionally built by both training-mode branches right after good/best), an independent field not derived from good/best's own sizes. Regression test extended with an equally-shortened case. - InstanceSpace.m: runSifted()'s density-resubsetting branch unconditionally dereferences obj.model.data_dense.X/Y/Ybin once obj.model.prelim.bydensity is true, but that's conditional -- doesn't belong in StageRequiredFields' unconditional 'sifted' list (would wrongly demand it for every model). Guarded locally instead, matching this class's own stated convention for conditional-path fields. Regression test added building a genuinely density-subsetted model. - output/scriptpng.m: rebuilding the same rootdir as 3D (or without CLOISTER) left behind a stale distribution_boundary.png from an earlier 2D build instead of removing it, so it looked like it belonged to the current result. Now deleted when the boundary isn't applicable. Minor: "vertice(s)" grammar fix in the ISA:scriptfcn:boundaryHoleOmitted warning; "test.unittest" typo fixed to "matlab.unittest" in tests/testRepoRoot.m's comment. Not changed: a suppressed comment claimed traceOneRegion (fixed last round, 4c31755) still doesn't stop correctly on a hole. Hand-traced it again against the actual committed code and confirmed the fix is correct as committed -- this appears to be a stale finding not accounting for 4c31755's already-applied nxt(1)==order(1) stop condition, not a real remaining gap.
Owner
Author
|
Addressed the fifth review round's other findings in 1485d61, alongside the posted
On the CI is running on 1485d61 now. Generated by Claude Code |
andremun
pushed a commit
that referenced
this pull request
Aug 21, 2026
Downloads badge is fully self-contained (shields.io reads the GitHub API directly, no setup needed). The Codecov badge will show "unknown" until two things happen: CI (which produces coverage.xml, currently only on 0.9.1/development pending PR #51's merge) actually uploads it to Codecov, and the repo is linked on codecov.io with a CODECOV_TOKEN secret added to the GitHub repo -- neither of which this commit does.
Feeds the Codecov badge added to README.md on docs/matlab-ci-testing-skill. fail_ci_if_error: false so a Codecov outage or a not-yet-configured CODECOV_TOKEN secret can't fail the actual test run -- pass/fail already happened above this step. The token secret itself and linking the repo on codecov.io are still needed on the codecov.io/GitHub side before the badge shows real data; neither can be done from a commit.
Owner
Author
|
Added a small scope addition in 3ae57f7: uploads Generated by Claude Code |
Filed after scoping a MATLAB-toolbox-style custom doc + GitHub Pages reference site at the user's request (~29 pages: 22 function/class reference pages, 7 conceptual/guide pages). Sized for v0.9.2 alongside #30/#52, not v0.9.1, despite not being an algorithmic change -- it's a genuinely new deliverable, not an engineering-quality fix.
This was referenced Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the
v0.9.1milestone: engineering-quality, architecture, and infrastructure follow-ups to v0.9.0, deliberately scoped to no algorithmic changes and no new user-facing options. Full details, organized as New functionality / Better engineering / Bug fixes, are in the newRELEASE_NOTES.mdv0.9.1 section added by this PR (the existing v0.9.0 section is untouched).Highlights:
onStageinspection callback forbuild()/explore()(Callback-based per-stage inspection in build() #26, Callback-based per-stage inspection in explore() #27); CLOISTER's boundary is now actually rendered for 2D projections (distribution_boundary.png,plot('boundary')) (Implement CLOISTER boundary display (deferred since refactor plan v1.7, never closed) #32).INIT.mtrain/eval data-ingestion function (Give INIT (data ingestion) a PYTHIA/TRACE-style train/eval dual-mode function, not a private method #38, supersedes Split data ingestion out of runPrelim #25/Unify PRELIM.m and evaluateTestSet's binary-performance computation (confirmed drift, not just duplication) #37); per-stage input/output contract validation (Per-stage input/output contract validation #28); CI added (Add CI to the MATLAB repository (currently none) #34);test_integration.mmigrated tomatlab.unittest(Adopt matlab.unittest instead of a plain script #39);SECURITY.md/CONTRIBUTING.mdadded (Add SECURITY.md and CONTRIBUTING.md #36); LIBSVM MEX-file provenance resolved by removal (Resolve LIBSVM MEX-file provenance #29);FILTER's previously-discarded diagnostic outputs kept (FILTER.m computes two outputs (isDissimilar, isVISA) that are never consumed anywhere #43); a few other small doc/dead-code cleanups.traceAlphaBoundarymulti-region export fixed (Investigate alpha-shape auto-retry for traceAlphaBoundary #31);opts.general.seednow actually reaches PILOT/SIFTED (Thread opts.general.seed through PILOT/SIFTED/PILOTviewpoint (and one more site) instead of rng('default') #41);ISAmigrateModel's LIBSVM-retraining path fixed to use raw performance data (ISAmigrateModel's LIBSVM retraining path passes normalized Y instead of Yraw to PYTHIA #42); CLOISTER warns on non-mean-centred data (CLOISTER.m's correlation-contradiction filter silently degrades when opts.prelim.norm=false #44);opts.perf.epsilonvalidation fixed forAbsPerf=true(ISAvalidateOpts.mapplies the[0,1]range check toperf.epsiloneven whenperf.AbsPerf=true#47).17 milestone issues are closed on GitHub already (each closing comment cites the commit that fixed it) — this PR is what actually lands that code on
master. Two items were deliberately deferred, not included here: #30 (PILOT projection-orientation canonicalisation) and #50 (CLOISTER's 2D-only convex hull for 3D projections), both algorithmic changes out of this milestone's scope, left for a futurev0.9.2. #35 (cross-repo Python-port fixture export tool) was closed as not planned — that concern belongs in thepyInstanceSpacerepo, not here.Also bumps
Contents.m/CITATION.cffto version 0.9.1, and fixesCITATION.cff'slicensefield (was an unrecognized SPDX identifier, causing v0.9.0's Zenodo archival to silently fail) to uselicense-urlinstead.Test plan
227b799, run 32350513845) —example.m+ the fullmatlab.unittestsuite (tests/*.m), 40/40 cases passing.ClassApiTest.testOnStageCallback(bothbuild()'s full-pipeline and'stages'-subsetonStagemodes, andexplore()'s), plus existingRegressionTestcoverage for the bug fixes listed above (seed reproducibility, CLOISTER non-mean-centred warning, required-field validation, TRACE good/best mismatch guard, PRELIM tie-breaking consistency, boundary display 2D/3D,traceAlphaBoundarymulti-region).Generated by Claude Code