Codex/open issue big rocks - #318
Closed
ybguzel wants to merge 111 commits into
Closed
Conversation
…AUDE.md Commits the planning documents that guided Phase -1 (fork merge) and scope the near-term (P/Q/T) and long-term (F/R) work ahead, so they're a shared source of truth instead of chat-delivered files.
…HIA mutating y_raw Closes #222 (P6) and #229 (P8) from the Phase P roadmap tracking issue (#241). - Metadata.from_data_frame no longer leaks the "feature_"/"algo_" CSV-column prefix into feature_names/algorithm_names, which fed directly into graph labels, CSV headers, and filenames. Confirmed against tests/matlab_reference fixtures, which already expect bare names (e.g. CART, not algo_CART). - _serialisers.py's z-space axis labels now use matplotlib mathtext ($z_{1}$/$z_{2}$) so the subscript actually renders instead of showing the literal text "z_{1}". - PythiaStage._generate_summary now copies y before mutating it in place, matching the existing pattern for its y_full/y_svms siblings. y was the same array object as the caller's y_raw, so every build() call silently poisoned the caller's data with NaNs. The mutated y was never read again afterward, so this changes no computed output, only removes the caller-visible side effect. Behavior-changing: any config using the old prefixed convention for selvars.feats/selvars.algos (e.g. "algo_CART") needs updating to the bare name to keep matching. No existing test or fixture relies on the old convention. Verification: targeted runs of test_serialisers.py, test_load_file.py, test_preprocessing.py, test_prepro_n_prelim.py, build_explore_adapter/, and the new regression tests all pass. Full-suite poetry run pytest triggered separately to confirm no wider regressions.
…/P1/P3/P4/P5) Closes P0, P1, P3, P4, P5 from the Phase P roadmap tracking issue (#241). P0 — dependency security hygiene: - Bump pillow (12.2.0->12.3.0), tornado (6.5.5->6.5.7), click (8.1.7->8.4.2), jupyter-core (5.7.2->5.9.1) via `poetry update`; pyproject.toml constraints already permitted these versions, so only poetry.lock changed. - pip-audit confirms zero known vulnerabilities in these four packages post-bump (was 26 CVEs across them); the only remaining audit hit is `pip` itself, which isn't a project dependency. - Added .github/dependabot.yml (pip ecosystem, weekly) so future drift is caught automatically instead of manually. - Added a non-blocking pip-audit CI step to validation-tests.yml. P1 — baseline hygiene: - README Contact section now points at this repo's own issue tracker instead of the MATLAB repo's. - Resolved the bare "TBD" citation placeholder using the repo's existing Zenodo concept DOI; added CITATION.cff (reusing the same DOI, matching MATLAB's schema). - Rewrote the PYTHIA options section to describe the actual scikit-learn SVC + grid/Bayesian-search implementation and current option names, instead of MATLAB's Statistics and Machine Learning Toolbox/LIBSVM. P2 — notebook parity: verified, no changes needed. liveDemoExploreIS.ipynb already explains why each stage matters and how to interpret its output (recent per-stage-diagnostics work), and already cross-references docs/explore_validation.ipynb bidirectionally. P3 — README structural parity: - Added "Repository layout" and "Working with the code" sections. - "The metadata file" section already existed; left as is. P4 — release notes discipline: added RELEASE_NOTES.md, seeded with a baseline entry for the current architecture plus an "Unreleased" entry covering this session's fixes. P5 — docs CI honesty: removed the static "docs-passing" badge (no CI job ever produced it) in favour of the existing "run pdoc instancespace locally" instructions already documented below it. No pipeline code touched by this commit; verification is docs/config only.
Per closer comparison against andremun/InstanceSpace's README.md and liveDemoIS.m (P2/P3): - Folded "The explore() inference pipeline" into "Working with the code" as a single subsection, rather than two separate top-level sections covering the same build()->explore() workflow. - Added the University of Melbourne DYA grant (2025DYA013) to Acknowledgements, matching the MATLAB repo's funding list. - Fixed the live-demo filename reference (liveDemoIS.m, not .mlx) while in the area. Notebook restructuring (liveDemoExploreIS.ipynb closer to liveDemoIS.m's stage-by-stage build() walkthrough) is in progress separately; this commit is the README-only portion so it isn't held up by that.
…lkthrough
Closer to MATLAB's liveDemoIS.m (P2), per andremun/InstanceSpace's README.md
and liveDemoIS.m as reference, and reusing proven code patterns from this
repo's own docs/explore_validation.ipynb (Part 2):
- Broadened the intro to cover both build() and explore(), matching
liveDemoIS.m's scope, rather than explore()-only.
- Replaced the single suppressed build() call with a full stage-by-stage
walkthrough: a Python-stage <-> MATLAB buildIS.m correspondence table,
the actual dependency-order printout (space._runner._stage_order),
SIFTED/PILOT/PYTHIA summary stats, a PILOT+CLOISTER visualisation
(training instances coloured by num_good_algos, CLOISTER boundary
overlaid), a TRACE footprint summary table, and a save-the-model step
(save_to_csv/save_graphs).
- Removed a now-dead `algo_label.replace("algo_", "")` workaround in the
PYTHIA test cell — no longer needed now that P6 strips the metadata.csv
column prefix at the source.
Column names used (Algorithm/Area_Good/Density_Good/Purity_Good on
trace.summary, Algorithms/CV_model_accuracy on pythia.summary) verified
directly against instancespace/stages/trace.py and pythia.py source,
not guessed.
Note: this commit captures the notebook's source cells; end-to-end
execution (via nbclient, to populate real outputs and confirm it runs
without error — build() takes several minutes on the trial dataset) is
running separately and will follow in a subsequent commit once confirmed.
SiftedStage._sifted() computed idx = np.arange(nfeats) once at the top of the method (using the pre-selection feature count) and returned that same stale value from every SiftedOutput(...) branch, instead of the narrowed selvars produced by feature selection/clustering. Downstream consumers (_serialisers.py's save_instance_space_to_csv, and FeatSel built from the same "idx" stage-output key) index data.x_raw by this field expecting it to match the already-reduced feat_labels/selvars — causing a shape mismatch (and silently wrong columns in the web-export path) whenever SIFTED actually narrows the feature set. Behavior-changing: any caller whose dataset causes SIFTED to reduce the feature count will now get correct (previously shape-mismatched or mis-aligned) feature_raw.csv / web feature-color export output. Verified against tests/exploreIS/sifted/ and tests/test_serialisers.py (17 passed) and by exercising Model.save_to_csv on a live build() run that performs real feature reduction, which previously raised "ValueError: Shape of passed values is (212, 10), indices imply (212, 6)" and now completes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
liveDemoExploreIS.ipynb's "Saving the model" cell writes CSVs/graphs to ./output by default; without this it shows up as untracked clutter in the working tree every time the notebook is run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
Re-executed the notebook after the SIFTED idx/selvars fix (a85c10f), which was blocking the "Saving the model" cell added in f62269a (Model.save_to_csv raised a shape-mismatch ValueError on any dataset where SIFTED actually narrows the feature set, as this notebook's does). All 25 cells now execute cleanly end to end with real output. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
… old live demo - README: rewrite PYTHIA/TRACE/parallel-processing sections to describe the actual Python implementation (scikit-learn SVC, shapely/alphashape, multiprocessing/n_jobs) instead of framing them as "replaces MATLAB's X"; keep genuine cross-codebase comparisons (explore()/exploreIS.m correspondence, liveDemoIS.m counterpart, matlab_reference validation, MATLAB-exported model format, uselibsvm backward-compat note). - README: "developed as part of the subject" -> "partly developed as part of the subject". - README/CITATION.cff: replace the placeholder code citation with the actual Zenodo/GitHub record citation and add the SoftwareX paper (DOI:10.1016/j.softx.2025.102246) as a second citation. - Move the previous liveDemoExploreIS.ipynb to docs/ ahead of rebuilding it from the MATLAB liveDemoIS.m structure (follow-up commit). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
…ff citation - README Options section mixed MATLAB struct-casing (opts.parallel.ncores, opts.sifted.NTREES, opts.cloister.cthres, opts.trace.PI, ...) with the already-fixed PYTHIA section's Python snake_case. Rewrite every remaining bullet to the actual Python dataclass field name, and fix opts.general. betaThreshold -> opts.perf.beta_threshold (there is no "general" options section in Python; beta_threshold lives on PerformanceOptions). Add a one-line note that options.json keys are matched case-insensitively and that legacy MATLAB-style spellings are still accepted for backward compatibility. - CITATION.cff: the top-level `authors` list (the 9 student contributors) had no relation to the citation text README.md actually asks readers to use (M.A. Muñoz and K. Smith-Miles). Add that exact citation as a `preferred-citation` entry so the two stay consistent, leaving `authors` as the contributor list and the SoftwareX paper under `references`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
…ructure Rewritten from scratch using liveDemoIS.m as the section-by-section outline (title/overview, licence/disclaimer, installation, then one section per pipeline stage, then post-processing, then explore()), with Python code substituted throughout and text adapted where Python's API genuinely differs (single build() vs MATLAB's incremental per-stage calls; explore() taking a Metadata object rather than a directory). Each build-time stage section calls next() on InstanceSpace.run_iter() once and inspects that stage's own raw output directly, mirroring integration_demo.py's explicit ordered Stage-class list rather than reading fields off the fully-assembled Model after one opaque build() call. CLOISTER and PYTHIA depend only on PILOT's output, so the scheduler may run either first; both results are pulled order-independently before either section is shown, rather than assuming a fixed order. End-to-end execution is running separately and will follow in a subsequent commit once confirmed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
PythiaOutput's docstring says "summary" but the real NamedTuple field is pythia_summary (execution caught this: 'PythiaOutput' object has no attribute 'summary'). Verification of the full notebook run is still pending; a follow-up commit will land once confirmed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
InstanceSpace.run_iter() (the public wrapper) declares its return type as None -- unlike StageRunner.run_iter()'s internal dict return -- so draining it via StopIteration never yields the assembled arguments dict; that's a deliberate part of its public API, not a bug to fix. Instead, merge the stage outputs already captured during the walkthrough (in the order they ran) into space._final_output directly, which is what build() does internally anyway, without paying for a second full pipeline run just to populate space.model. Verification of the full run is still pending. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
All 41 cells run cleanly with real outputs: the stage-by-stage build() walkthrough, save_to_csv()/save_graphs(), explore(), and the explore_iter() inference walkthrough with its three plots. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
Updated README.md for clarity and consistency in language, corrected minor grammatical issues, and improved formatting for better readability.
Removed multiple authors and added a new author Muñoz with given name Mario Andrés.
…ch fix Commit a85c10f fixed the shape-mismatch crash by aliasing idx=selvars at every SiftedOutput construction site, but left two fields (idx, selvars) permanently identical with no distinct purpose. Root-cause fix: remove idx entirely from SiftedOutput (instancespace/stages/sifted.py) and from SiftedOut (instancespace/data/model.py, Model.sifted) - both were always identical to selvars in this codebase, so keeping both fails the "why are there two of these" test. FeatSel.idx (its only field) stays, now sourced from stage_runner_output["selvars"] instead of the removed "idx" key. Fixed every consumer found by grepping the whole repo, not just the two crashing call sites in _serialisers.py: - FeatSel.from_stage_runner_output and SiftedOut.from_stage_runner_output (instancespace/data/model.py) - Both _serialisers.py call sites (data.x_raw[:, sifted_out.selvars] and data.x_raw[:, feat_sel.idx], the latter unchanged since FeatSel's field name is unaffected) - 6 test fixture constructions across test_sifted_unit.py, test_sifted_validation.py, and test_serialisers.py - docs/explore_validation.ipynb's SiftedOut(...) construction Removing the redundant field surfaced a second, previously-masked bug: test_serialisers.py's SiftedOut fixture built `selvars` directly from raw 1-based MATLAB data without the "-1" conversion `idx` used to have - invisible before because _serialisers.py always indexed with the correctly-adjusted `.idx`, never the raw `.selvars`. Fixed alongside. Verified: full pytest suite (245 passed, 1 pre-existing unrelated failure in test_cloister.py, confirmed against the pre-session baseline commit earlier this session); both docs/explore_validation.ipynb and liveDemoExploreIS.ipynb re-executed end-to-end with 0 errors and identical comparison numbers to before this fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
…to v0.9.0/development-branch-P0
…emoIS.ipynb rename Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
…ter (Q1) _svc_to_artifact() only handled "rbf" and "linear"; calling explore() after a build() trained with opts.pythia.is_poly_krnl=True raised NotImplementedError. The tuned gamma PYTHIA trains poly-kernel SVMs with can't be expressed via _explore_pythia's existing polynomial formula (z . sv + 1)**degree, which hardcodes gamma=1 (matching PYTHIA's own hardcoded coef0=1). Support vectors are pre-scaled by gamma instead, so (gamma * z . sv + 1)**degree is computed as ((z . (gamma * sv)) + 1)**degree - verified numerically to reproduce scikit-learn's actual decision_function to float precision. Also fixed test_unsupported_kernel_raises, which used kernel="poly" to test the unsupported-kernel path; switched to "sigmoid" and added dedicated poly coverage (decision function transfer, kernel_param/degree, explore_pythia posterior reproduction) using a poly SVC trained with the same degree=2/coef0=1 PYTHIA itself uses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
…methods, feature-order docs (Q2/Q4/Q5/Q7) Bundled together since all four touch instancespace/instance_space.py and are small, additive, low-risk quality items: - Q2: explore() now logs a loguru warning (via _explore_prelim) when >5% of test instances have a feature outside the training PRELIM bounds and get clipped, matching MATLAB's equivalent check. Threshold is a named module constant, not a bare literal. Tests in tests/exploreIS/prelim/test_prelim_unit.py cover both the firing and silent cases via a loguru sink. - Q4: instance_space_from_files' options listing now recurses into nested option dataclasses (instancespace/utils/print_options.py::format_options), printing one line per leaf field (e.g. "parallel.flag") instead of one line per top-level group with a raw nested-dataclass repr. - Q5: documented (explore()'s docstring) and added a regression test for the already-decided permissive feature-order behaviour: test metadata's feature columns are matched by name, not position, so may be supplied in any order. - Q7: added plot_sources()/plot_portfolio()/plot_good()/plot_footprint() convenience methods (instancespace/plotting.py), thin matplotlib wrappers mirroring MATLAB's InstanceSpace.plot(view, algoIdx). Per the issue's recommended default, these are four separate methods rather than one dispatch method taking a view-name string - flagging that choice here in case the single-method MATLAB-mirroring signature is preferred instead. Verified: tests/build_explore_adapter/, tests/exploreIS/, tests/test_pythia.py (62 passed), plus tests/test_load_file.py/test_prepro_n_prelim.py/ test_preprocessing.py (128 passed, covering instance_space_from_files) and the new test_plotting.py/test_print_options.py/test_extract_features.py. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
SECURITY.md points at GitHub private vulnerability reporting. CONTRIBUTING.md points at the README's existing setup guide rather than duplicating it, and notes that poe test does not currently run pytest (T4, not done yet) so both commands are needed until that's fixed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
Wilson et al.'s "Best Practices for Scientific Computing" (2014), practice 2h: don't comment/uncomment code to control behaviour. Enabling it as-is would fail CI on every push (314 ruff errors + 123 mypy --strict errors + 16 files needing black reformatting exist across the repo today, unrelated to any current change) - confirmed by actually running all three before deciding. Removing the dead lines resolves the anti-pattern now; full lint/type enablement is a separate, larger future task given that backlog. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
New opts.general.* group, mirroring MATLAB's namespace, added as the last field on InstanceSpaceOptions with a default_factory so every existing direct InstanceSpaceOptions(...) construction and every InstanceSpaceOptions.default( *([None] * 12)) call (used throughout the notebooks and tests) keeps working unchanged - both fields default to exactly today's implicit hardcoded behaviour (verbose=True, seed=0), so introducing the option changes nothing for any existing caller. Not yet wired into any stage - that's Q3 (print()->logger, gated behind general.verbose) and Q9 (threading general.seed through pilot/sifted/prelim/ pythia's hardcoded seed=0/random_state=0 call sites), which follow. Verified: existing options/load_file/serialisers/manual_selection suites (137 passed) plus 6 new tests (default values, backward-compat positional and direct construction, from_dict parsing including a null seed for non-deterministic runs). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
Completes Q9's RNG-seed centralisation (GeneralOptions.seed already added in a prior commit alongside prelim.py's threading). Parameterizes the remaining 7 hardcoded seed=0/random_state=0 call sites across pilot.py, sifted.py, and pythia.py into general_options.seed, threaded through each stage's Input NamedTuple so the DAG scheduler resolves it from InstanceSpaceOptions.general. Default seed is 0, exactly matching every previously-hardcoded value, so this is additive: no existing caller's output changes. Verified via: - Full pytest runs of test_pilot.py, test_pilot_pythia.py, exploreIS/pilot/, test_sifted.py, exploreIS/sifted/, and test_pythia.py (0 regressions). - New reproducibility tests per stage: same seed -> identical output, different seed -> different output. - Re-executed docs/explore_validation.ipynb end-to-end (0 errors, all MATLAB-comparison numbers unchanged), proving the DAG resolves the new general_options field correctly across prelim/pilot/sifted/pythia together. - mypy --strict and ruff clean on every touched file. Scope note: this does not implement MATLAB's per-fold/per-trial reseeding discipline (foldSeed = baseSeed*1e5 + fold*1e3) for SIFTED's GA fitness evaluations or PYTHIA's per-algorithm tuning loop - the existing "one rng flows through the whole loop" structure is preserved, just parameterized. Issue #257's stated acceptance criteria (same-seed reproducibility, different-seed difference, default-seed bit-identical to pre-change output) don't require the fuller redesign, which would carry materially higher risk to this statistically-sensitive code with no existing test coverage to lean on. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
Replaces all 121 unconditional print() calls across instancespace/stages/*.py and instance_space.py with logger.info/logger.debug/logger.warning/logger.error calls, adopting a "[STAGE] message" prefix convention. Per-trial/per-iteration detail (e.g. PILOT's per-trial BFGS message, TRACE's pairwise footprint comparisons, SIFTED's per-cluster suggestions) is now gated behind `general_options.verbose`, defaulting to True so nothing disappears by default; top-level stage narrative always logs regardless of verbose. pilot.py, prelim.py, sifted.py, pythia.py, trace.py, and cloister.py each gained (or reused, where Q9 already added it) a `general_options: GeneralOptions` field on their Input NamedTuple, resolved automatically by the DAG scheduler via _InstanceSpaceInputs. preprocessing.py and cloister.py needed no verbose gating (no per-iteration detail exists in either), so no threading was added there, per the "don't add options nothing consumes" principle. Incidental but necessary fix: data/metadata.py's and data/options.py's own error-reporting print() calls (companions to instance_space.py's "Failed to initialize metadata/options" messages, part of the same error-reporting flow) were also converted to logger.error - leaving them as print() while their caller became a log call would have produced inconsistent, split stdout/stderr output and broken every test in test_load_file.py that asserts on that combined error text. Discovered while fixing those tests: loguru's default sink binds sys.stderr once at import time, so pytest's capsys fixture (which patches sys.stderr per-test) never sees loguru's output - capsys.readouterr().err stays empty even though pytest's own capture shows the text. All capsys-based assertions in test_load_file.py were switched to a loguru-sink capture helper (_collect_error_logs), matching the pattern already used for the Q2 OOD warning test. Verified via: - Full pytest runs of test_pilot.py+test_pilot_pythia.py+exploreIS/pilot/ (18 passed), test_sifted.py+exploreIS/sifted/ (16 passed), test_pythia.py (9 passed), test_trace.py (2 passed), test_load_file.py (126 passed), test_prelim*.py+exploreIS/prelim/ (20 passed), test_preprocessing.py (1 passed), plus a new test_verbose_logging.py smoke test asserting per-trial detail only appears when general.verbose is True. - Re-executed docs/explore_validation.ipynb and liveDemoIS.ipynb end-to-end (0 errors in both, all MATLAB-comparison numbers unchanged), proving the DAG resolves general_options correctly across the full pipeline and that no caller's output changed. - mypy --strict, ruff, and black clean on every touched file. One pre-existing test failure (test_cloister.py::TestCloister::test_run, a convex-hull sign/ordering mismatch) was confirmed present and identical on the unmodified branch before this change - unrelated to this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
[Additive] test_data/demo/ was a multi-dataset collection of real, runnable example data (BBO/JSS/KP/MFP/MGP/MOCBBO/T1/T2/gcp/tsp + options_*.json), used by integration_demo.py/example_plugin.py - not a MATLAB-comparison test fixture, but its location under test_data/ implied otherwise (one of the sources of confusion the parent test- data audit set out to resolve). Moved via git mv (content and history preserved) to examples/data/, with both real readers' path strings updated in the same commit. Corrected the issue's own premise before executing it: liveDemoIS.ipynb does not read test_data/demo/ at all (it reads tests/matlab_reference/input/) - the two actual readers are integration_demo.py and example_plugin.py, confirmed by grep rather than trusting the issue text. Verified, not just moved: ran both scripts against the relocated path. Both resolve and read examples/data/options.json successfully, proving the move itself is mechanically correct; both then fail at options validation on a genuine, pre-existing bug unrelated to the move (selvars.type: "Ftr&&Good", confirmed present at the same value before this commit) - filed separately as #311 rather than silently fixed, since this step is relocation-only, no fixture content changes. pytest --collect-only confirms unaffected (416 tests, unchanged). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
[Additive] "Ftr&&Good" (double ampersand) -> "Ftr&Good". Found while verifying T10d's relocation of test_data/demo/ to examples/data/ - confirmed pre-existing (same value at the same path before that move), not caused by it. Verified end-to-end, not just past the validation step: both integration_demo.py and example_plugin.py now run to completion, producing TRACE's full footprint summary table. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
[Additive] New optional fields on SiftedOptions and CloisterOptions, each defaulting to today's existing hardcoded behaviour, so no existing caller's output changes. - SiftedOptions.pval (default 0.05): the correlation filter's significance threshold, previously a hardcoded SiftedStage.PVAL_THRESHOLD class constant, now configurable to match MATLAB's opts.pval (core/SIFTED.m). #300 audit finding, issue 2. - SiftedOptions.dims (default 2, validated to {2, 3}): the projection dimensionality the GA fitness function's internal PILOT call uses for its KNN neighbour count (kneighbours = dims + 1), matching MATLAB's opts.dims. PILOT itself is 2D-only in this port (3D is F2's unshipped future work), so dims=3 is accepted for forward API compatibility but has no effect on PILOT's actual output yet. #300 audit finding, issue 4. - CloisterOptions.hull_dims (default "all"): restricts the convex hull's geometry computation to the first N projected columns while still returning full-dimensional vertices, letting callers opt into MATLAB's always-2D-hull behaviour (core/CLOISTER.m). #299 audit finding, issue 5. Both _check_sifted_dims and _check_cloister_hull_dims are wired into InstanceSpaceOptions.__post_init__, and the docstring's "MATLAB fields with no Python equivalent yet" list is updated to reflect these three fields no longer belonging there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
…nd dims [Behavior-changing] SIFTED's GA-driven feature selection changes: the GA fitness cache eliminates redundant PILOT+KNN evaluations for already-seen feature subsets (same result, faster), but the analytic-PILOT fix and the dims+1 neighbour count change what PILOT/KNN actually compute inside the fitness function, so the GA's search trajectory - and therefore SIFTED's selected feature set - can change output. Verified via the full SIFTED, PYTHIA, and CLOISTER test suites (all passing, no non-deterministic failures across repeated local runs) and the project's existing seed-reproducibility tests, which still pass; no MATLAB reference run was available this session to confirm closer numeric agreement, so this is not a claim of exact parity, only that no regression was introduced against this repo's own existing test suite. Root-caused against MATLAB's core/SIFTED.m, #300 audit findings (issues 2, 4, 6): - Issue 2: `insignificant_pval = pval > self.PVAL_THRESHOLD` used a hardcoded class constant instead of `self.opts.pval`, ignoring the option MATLAB's opts.pval is meant to control. Now uses the new SiftedOptions.pval field (previous commit). - Issue 4: the GA fitness function's internal PILOT call used `PilotOptions.default()`, whose own default is `analytic=False` - a real bug, since MATLAB's costfcn hardcodes `analytic=true, ntries=5` specifically for this hot path (it runs once per GA candidate, hundreds of times per SIFTED call), independent of whatever PilotOptions the outer pipeline actually uses. Also replaced the hardcoded `K_NEIGHBORS = 3` KNN neighbour count with `dims + 1`, matching MATLAB's `kneighbours = dims + 1` (dims now threaded from the new SiftedOptions.dims field). - Issue 6: `cost_fcn` recomputed PILOT+KNN from scratch on every GA evaluation, even for a feature-selection bitmask already seen in an earlier generation. Added a cache keyed by the bitmask (`idx.tobytes()`), scoped to the `ga_instance` object for this SIFTED call only (not a MATLAB-style cross-call persistent map) - correctly isolated per worker process under `parallel_processing=["process", n]`, since pygad pickles a separate ga_instance per worker, matching (not regressing from) MATLAB's own documented per-worker persistent-variable limitation. Issues 1, 3, 5, 7, 8, 9 from #300 are not addressed here: issues 5 and 7 were explicitly held back for a separate design decision (GA fitness metric and clustering distance semantics); issues 1, 3, 8, 9 remain open on #300, not silently dropped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
[Behavior-changing] When use_weights=True and the classifier supports sample_weight, cross_val_predict's internal fold fits during hyperparameter candidate ranking are now weighted - previously only the final full-data fit was weighted, leaving Sobol/Bayes candidate selection itself blind to sample weights. This can change which hyperparameters are selected as best for a weighted PYTHIA run; unweighted runs (use_weights=False, the default) and classifiers that don't support sample_weight are unaffected. Verified against tests/test_build_pythia.py (36 passed) and the full project test suite (420 passed), including test_pythia_use_weights_degenerate_falls_ back_to_uniform and the KNN-ignores-weights warning test, with no regressions. Root-caused against MATLAB's core/PYTHIA.m, #298 audit finding, issue 6: MATLAB threads Wtrain into every CV fold's fit (evalFoldClassifier -> fitOneClassifier) for both Sobol- and Bayes-candidate ranking, not just the final fit. Verified directly against sobolSearch's own ranking metric (errs = mean(Ysub_all ~= Ybin_rep, 1)) that only the fit is weighted, never the aggregated misclassification-rate/error metric itself - so the new _cv_fit_params helper only ever threads sample_weight into cross_val_ predict's fit step, matching MATLAB precisely rather than inventing a weighted-error-metric interpretation. cross_val_predict's fit_params= kwarg is deprecated in this project's installed sklearn (1.5.2, removed in 1.6); all four call sites use the newer params= kwarg instead, confirmed to work without needing sklearn.set_config(enable_metadata_routing=True). Issue 10 (eval mode / skip mode) remains explicitly deferred to F8/F9, not addressed here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
[Additive] hull_dims defaults to "all" (today's existing unrestricted behaviour) - no existing caller's output changes. Setting it to an integer is new, opt-in behaviour with no prior callers to regress. _compute_convex_hull gains an optional hull_dims parameter: when given, the hull geometry itself is computed on the first hull_dims columns of the projected points, but the returned vertices still carry every column (mirroring MATLAB's core/CLOISTER.m, which always builds a 2D hull on the first two projected columns regardless of how many columns A has, yet keeps full-dimensional vertex coordinates). Wired into all three hull call sites inside cloister() via the new CloisterOptions.hull_dims field (previous commit). hull_dims exceeding the point set's actual column count degrades gracefully (NumPy slicing past an array's width is a no-op) rather than raising. #299 audit finding, issue 5 (the only remaining item on #299 - issues 1-4 were already fixed in earlier commits this session). PILOT's projection is 2D-only in this port (3D is F2's unshipped future work), so hull_dims="all" and hull_dims=2 are currently equivalent in practice - documented in the option's own docstring, not silently assumed obvious. Adds 4 new tests per the issue's own acceptance criteria: hull_dims=None matches the unrestricted default, hull_dims=2 restricts geometry while keeping full-dimensional output columns, hull_dims exceeding the column count doesn't crash, and an end-to-end cloister() run with hull_dims=2 still matches the MATLAB reference fixture. Verified: tests/test_build_ cloister.py (19 passed) and the full project test suite (420 passed), no regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
Adds v1.57 (records the earlier #311 typo fix, commit 35105a0, and the scope decisions made via AskUserQuestion for #298/#300/#301 before this session's implementation work started) and v1.58 (this session's SIFTED #300 issues 2/4/6, PYTHIA #298 issue 6, and CLOISTER #299 issue 5 fixes) to the document-history table, and updates §6.3's per-stage status bullets and priority note to match - CLOISTER (#299) now has no findings left open. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
[Additive] Pure type-annotation fix, no behavior change. The GA fitness cache added in commit 6731aa6 returned `instance.cost_cache[cache_key]` directly - since `instance` (a pygad.GA object) has no type stub, that expression is `Any`, and `mypy --strict` (this project's actual gate per `poe check_mypy` / `pyproject.toml`'s `check_mypy = "mypy --strict ."`) flags "Returning Any from function declared to return float". Missed earlier because that commit was verified with plain `mypy`, not `--strict`. Fixed by binding the cache lookup to an explicitly-typed local before returning it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
[Additive] Every item here was verified individually against the whole repo (grep across instancespace/, tests/, docs/, notebooks) before removal, not deleted on vulture's raw output alone (which had plenty of false positives - NamedTuple/dataclass fields consumed reflectively, public API methods, active enum members - all left untouched). Full test suite passes unchanged (420 passed), coverage rose from 91.33% to 93.30% since the removed code was 0%-covered dead weight dragging the average down. Removed: - instancespace/scripting/ (script_fcn.py, script_disc.py, __init__.py) entirely, per explicit user decision after flagging the one ambiguous case: every function in script_fcn.py was a `raise NotImplementedError` stub with 0% coverage, and _serialisers.py already has working private equivalents (_draw_scatter, _draw_binary_performance, _write_array_to_csv, etc.) that superseded them. script_disc.py's citation/disclaimer printer was likewise unreferenced anywhere - CITATION.cff/README already serve that role. The roadmap's F5 item named these files as its future implementation target; updated F5's pathway note in docs/python_implementation_pathways.md to point at _serialisers.py instead, and recorded the audit finding F5's own pathway asked for (script_fcn.py's drawing functions are 2D-only throughout). - StageRunner.run_many_stages_parallel (stage_runner.py) - a `raise NotImplementedError` stub already flagged in the roadmap's F4 audit (Sec 6.1) as "unused dead code, not a regression" but never actually removed until now; updated that roadmap note to match. - MissingOptionsError (data/options.py) - custom exception class never raised or caught anywhere. - InstanceSpaceOptions.to_file() (data/options.py) and Metadata.to_file() (data/metadata.py) - both `raise NotImplementedError` stubs, never called. - PreprocessingOut (data/model.py) - a fully empty dataclass (`pass` body) - and PreprocessingDataChanged (data/model.py), both entirely superseded by stages/preprocessing.py's own PreprocessingOutput NamedTuple, never referenced anywhere else. - DEFAULT_PERFORMANCE_NUM_CORES (data/default_options.py) - an orphaned constant; PerformanceOptions has no field that uses it (parallelism already has its own DEFAULT_PARALLEL_N_CORES). - _write_cell_to_csv (_serialisers.py) - private helper never called (every call site uses _write_array_to_csv instead). - StageProgress dataclass + its to_dict() method (progress_reporter.py) - never instantiated anywhere; the actual progress reporters build dicts directly rather than going through this class. Not removed, flagged instead: StageStatus.PENDING (progress_reporter.py) is a currently-unassigned enum member (only RUNNING/COMPLETED/FAILED are ever set), but removing an enum value from a class documented as communicating status "to external systems" risks breaking a consumer's contract even though nothing in this repo emits it today - left in place rather than guessed at. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
Adds the document-history entry for the vulture-assisted dead-code removal (commits 4e6bc72, 28d7389), and updates F4's Sec 6.1 audit note (already flagged run_many_stages_parallel as unused dead code, now actually removed rather than left as a stub) and F5's pathway in python_implementation_pathways.md (its named files were deleted; points at _serialisers.py instead, with the 2D-only audit finding F5 itself asked for now recorded). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
…e (§7.1) [Additive] Documentation only, no fixture moved - same treatment as the rest of §7, still blocked on #278. Hashed every file under tests/test_data/ and tests/matlab_reference/ to find duplication the original file-existence-based audit wouldn't catch. Found two patterns beyond the already-decided build-vs-explore naming unification: - Pattern A (pipeline-chained): PRELIM's output is byte-identical across up to 9 private copies feeding SIFTED/PYTHIA/TRACE's own test fixtures, each with a distinct confirmed reader. Fix: downstream tests read the upstream stage's own build_data/<stage>/output/ directly - no new top-level category, already inside T10e's scope. - Pattern B (genuinely shared, no single producing stage): the same metadata.csv is byte-identical across test_data/load_file/, test_data/preprocessing/, and matlab_reference/input/, each with an independent reader. Fix: a new top-level shared_inputs/<name>/ directory (name open to revision). Also noted: tests/test_data/trace_csvs/ is real and used but doesn't follow the <stage>/input/,output/ convention every other stage uses - folded into T10e's scope, no separate action. Folded into GitHub issue #310 (T10e) now, per direct instruction, so the eventual migration moves fixtures once instead of twice. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
Splits PilotOptions.alpha (previously overloaded as a precomputed solution vector) into precalc_alpha (same meaning, unchanged) and a new cost_weight scalar matching MATLAB's costWeight, threaded through analytic_solve()'s Y-block scaling and error_function()'s weighted error (#301 issues 1/3/7). Defaults to 1.0, an exact no-op at that value, so this is additive rather than behavior-changing. Also parallelises numerical_solve()'s ntries restart loop on a ProcessPoolExecutor (empirically faster than a thread pool for this CPU-bound BFGS solve), guarded against nesting inside SIFTED's own GA worker processes via multiprocessing.parent_process() (#262/F2). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
…ustering Implements SIFTED audit findings #300 issues 5 and 7: - cost_fcn's k-NN fitness now scores accuracy and computes loss = 1 - accuracy instead of neg_mean_squared_error on the binary good/bad labels, matching MATLAB's fitcknn/kfoldLoss classification loss rather than treating a classification target as regression. - evaluate_cluster and select_features_by_clustering now cluster z-scored feature vectors (new _standardize_for_correlation_distance helper) before calling KMeans, reproducing a correlation-distance k-means's nearest-centroid assignment via the identity ||u-v||^2 = 2n*(1 - corr(u,v)) for population-z-scored vectors, matching MATLAB's kmeans(...,'Distance','correlation'). Closes the existing inconsistency with silhouette_score's own metric="correlation". Behavior-changing: SIFTED's selected feature set can change. Verified against the full test suite (423 passed) plus 4 new regression tests. Also corrects a stale bookkeeping comment on #300 (issues 1/3/8/9 were already fixed, not "untouched" as a prior comment claimed), and files #312 for a separately-discovered, unfixed concern: pygad maximizes fitness while MATLAB's ga minimizes, and cost_fcn's return value has no sign inversion - a bigger, independently-verification-worthy question not folded into this change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
pygad always maximizes cost_fcn's return value, but the previous loss-based formula (1 - accuracy, worst-case maximized) had the GA searching for the *worst* feature combinations instead of the best, with no sign inversion to correct it - matching MATLAB's ga(), which minimizes, would have required negating the loss. Sidesteps the sign question entirely: track and return the minimum per-algorithm accuracy directly (y = min(y, scores.mean())). This is provably equivalent to the correctly-negated loss formula (same optimum up to an additive constant), not just an alternative that happens to also work. Test updated to use two algorithms with different accuracy so the assertion actually distinguishes minimum from maximum. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
… algorithm Python's trace.py only implements MATLAB's legacy DBSCAN-based TRACE algorithm; TRACE3 (MATLAB's actual current default) was explicitly scoped out of F11 as "a separate, much larger future item if ever prioritised" but never turned into a tracked issue - filed now on request. Scoped against MATLAB's actual core/TRACE.m (read directly, not inferred): TRACEbuild3's alpha-shape-then-iterative-purity-tightening algorithm, TRACErescore's evaluation-mode counterpart, and the dispatcher logic. Documents the real roadblock - MATLAB's alphaShape object's alphaSpectrum/RegionThreshold/native-3D capabilities have no ready-made Python equivalent, only buildable-from-primitives via the alphashape package's lower-level alphasimplices/circumradius - and flags that exact MATLAB parity may not be achievable the way this repo's other ports are verified. No code changes; adds docs/python_implementation_pathways.md's ### F16 section, a roadmap table row + recommended-order entry, and files GitHub issue #313 as a Phase F (#260) sub-issue. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
…cope Adds PilotOptions.method="pls" (F2/#262's PLS alternative to the existing analytic/numeric solvers), using sklearn.cross_decomposition .PLSRegression(scale=False) to match MATLAB's plsregress preprocessing exactly. pls_solve() takes dims as a parameter and is verified working unmodified at dims=3, so a future public dims option needs no rework here - only 2D is exposed today. Caught a real correctness issue before shipping: MATLAB's out.A is documented as reprojecting new instances via Z=X*A', which holds for plsregress's SIMPLS algorithm but not for sklearn's NIPALS-based PLSRegression - x_weights_ doesn't satisfy that identity beyond the first component (~0.16 error), only x_rotations_ does (~1e-16). Uses x_rotations_ for out_a accordingly. Additive: method defaults to "standard", identical to today's behavior. Verified against the full test suite (433 passed, up from 427, no regressions) plus 6 new regression tests. Also corrects F8/F9's documented scope, verified directly against MATLAB's core/PYTHIA.m and InstanceSpace.m's evaluateTestSet: F8's "PYTHIA half resolved by S1" claim was wrong (_explore_pythia still duplicates _determine_selections/_compute_znorm), and F9's ground- truth evaluation scope was missing PYTHIA's own accuracy/precision/ recall computation against that ground truth. Docs-only correction, no code changes for F8/F9 themselves. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
…re() to ground-truth evaluation F8: extract PythiaStage._determine_selections's precision-weighted selection formula into a shared _weighted_selection() static method, called by both training (_determine_selections) and explore()'s _explore_pythia, eliminating the confirmed duplication; _explore_pythia also now reuses PythiaStage._compute_znorm() instead of recomputing the same formula inline. Audited TRACE against the same goal and found no live duplication to extract - _explore_trace() already reuses the trained footprint polygon rather than re-deriving it - but surfaced a separate contains-vs-covers point-in-polygon inconsistency (filed as #315) and a pre-existing nalgos==1 selection-index bug (filed as #314), neither fixed here since both are independent [Behavior-changing] fixes needing their own verification. F9: extract compute_binary_performance() from PrelimStage._prelim() into prelim.py, shared with a new InstanceSpace._explore_evaluate() that computes real ground-truth accuracy/precision/recall/confusion-matrix from PYTHIA's predictions, gated behind a new ExploreStage.EVALUATION yielded only when test metadata carries algorithm performance columns. ExploreResult gains 8 new Optional fields, all None in the feature-only case. Full test suite: 442 passed (up from 433), 4 pre-existing-slow tests deselected, no regressions. Closes #268, #269. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
…ty new-algorithm support PRELIM: extract apply_bound_clip()/apply_boxcox_zscore() as shared pure functions, used by both PrelimStage._bound()/_normalise() (training) and a rewritten InstanceSpace._explore_prelim() (explore) instead of a hand-duplicated second copy. Verified bit-for-bit equivalent to the code they replace before swapping. Two real bugs fixed as a direct consequence of writing the shared function correctly: _normalise() used plain np.min (not np.nanmin), which propagates NaN across an entire feature column from a single missing value; and _explore_prelim() ignored BoundOptions.flag/NormOptions.flag entirely, unconditionally clipping/normalising regardless of what the trained model actually used (unsafe when norm=False, since lambda_x/mu_x/sigma_x are unfit zero arrays in that case). F9: implement the previously-deferred "new algorithm absent from training" edge case at full MATLAB parity. _explore_pythia()/_explore_trace() gain an n_new_algos parameter widening y_hat/pr0_hat/in_good/in_best with False/0.0 placeholders (matching MATLAB's PYTHIAevalMode/TRACEthrow3 padding), not just the evaluation metrics. New algorithms participate as full candidates in y_best_actual/p_actual/beta_actual but report NaN accuracy/precision/recall/cvcmat (no trained classifier exists to score them). New ExploreResult.algo_labels field exposes the resulting widened algorithm order. 7 new tests. Full suite: 449 passed (up from 442), 4 pre-existing-slow tests deselected, no regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
F17 (#315, retitled): TRACE's _explore_trace() .covers()/.contains() mismatch, corrected direction confirmed via direct MATLAB source read (isinterior is boundary-exclusive in both TRACE_legacy.m and the explore-mode TRACErescore) - explore's .covers() needs to become .contains(), not training's .contains(). F18 (#316, new): unify build/explore into single-body stage methods for all stages, filed as a future architectural proposal, not implemented. Both linked as Phase F sub-issues of #260. No code changed - filing and doc sync only.
…E degradation PythiaOptions.skip bypasses classifier training entirely, matching MATLAB's opts.skip/emptyPYTHIAout: real zscore mu/sigma still computed, every classifier-derived output field becomes a "nothing trained" placeholder (_ConstantClassifier(False) per algorithm, -1 selection sentinels, a 9-column summary with no hyperparameter columns). Investigating this surfaced that this port's TRACE (legacy-only) clusters PYTHIA's y_hat predictions with DBSCAN to build compact footprints - y_hat fills the role DBSCAN's own density clustering plays on raw labels in true legacy TRACE. Skipping PYTHIA while trace.use_sim=True would silently degrade footprints from compact regions to fragmented ones built from raw, noisy y_bin, unlike MATLAB's trace3 (which has an independent, Yhat-free compacting mechanism and so degrades gracefully instead). Rather than ship that silently, InstanceSpaceOptions.__post_init__ now rejects pythia.skip=True combined with trace.use_sim=True outright. [Additive] - skip defaults to False, at which every existing caller's PYTHIA output is unchanged. Also files #317 (not fixed here): _generate_summary's accuracy/precision arguments are passed in swapped positional order, found while building skip mode's own call to that function. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013n7G3hragng5H6SgGCgPHF
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
3 participants
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
This PR addresses the largest verified correctness and reliability gaps between the Python implementation and the MATLAB reference.
It also adds concise architecture, remediation, implemented-fix, and pending-backlog documentation covering all 17 open issues reviewed during this pass.
Changes
Issue disposition
Fixes #302
Fixes #314
Fixes #317
#315 is intentionally not implemented. MATLAB
[polyshape.isinterior](https://www.mathworks.com/help/matlab/ref/polyshape.isinterior.html)includes boundary points, and the existing MATLAB fixtures confirm boundary-inclusive behavior. Changing Python to boundary-exclusive membership reduced fixture agreement.All other open issues were triaged in
docs/pending_issue_backlog.md. Large deferred work includes TRACE3, 3D PILOT/output support, MATLAB fixture provenance, and build/explore API unification.Validation
git diff --checkpasses.Compatibility notes
correct_results_simulation.csvis explicitly treated as a Python regression baseline, not a verified MATLAB oracle.