Feat/registry driven config#156
Merged
Merged
Conversation
Drives the real osipy CLI against the local datasets (DCE, IVIM brain/abdomen, ASL ExploreASL/OSIPI-1) plus GE/Siemens/Philips DICOM-load checks, validating every output NIfTI. Reproduces the 8 passing localdata checks used as the dead-code-removal regression baseline so they can be verified independently. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Invert the config dependency so the registry is the single source of truth.
Each component declares its own pydantic config model (a MethodConfig with a
'method' discriminator + its knobs); the CLI config is generated from
registry x schema, and components are constructed straight from a validated
config instance.
Core (osipy/common/config/):
- MethodConfig base (extra='forbid')
- method_union(configs): build a discriminated union from {name: config}
- construct_from_config(registry, cfg): instantiate the selected component
from its config (validation and construction share one schema -> a knob
can't be 'collected but ignored')
DSC slice (proves the pattern end-to-end):
- SSVDConfig/CSVDConfig/OSVDConfig + DECONVOLVER_CONFIGS/_REGISTRY
- DSCPipelineYAML.deconvolution is the generated discriminated union;
te/baseline_frames/hematocrit_ratio are real knobs now
- compute_perfusion_maps accepts an injected fitter; DSCPipeline auto-builds
it from config (per-method params actually take effect; previously the
fitter was default-constructed and svd_threshold was dropped)
- dump_defaults renders the nested config automatically (recurses models)
- wizard introspects the chosen method's config model to prompt its params
Gate: 825 passed, 0 failed (core incl. GPU); nested YAML round-trips via
load_config; unknown per-method keys rejected.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply the registry-driven config pattern to DCE (osipy/dce/config.py): PK model, T1 method (VFA fit_method linear/nonlinear now selectable), concentration (spgr/linear now selectable), and population AIF become generated discriminated unions in DCEPipelineYAML; DCEPipeline auto-constructs from the validated config. aif_source (pipeline branch) and fitter (in DCEFittingConfig) stay flat. dump_defaults renders nested; wizard introspects per-method params. Gate: 834 passed, 0 failed (core incl. GPU); nested DCE YAML round-trips. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply the registry-driven config pattern to ASL (osipy/asl/config.py): m0 calibration, difference method, and a new quantification *mode* (single_pld vs multi_pld) become generated discriminated unions in ASLPipelineYAML. Multi-PLD (Buxton + ATT) is now a selectable mode producing CBF + ATT maps. The ASL physiological knobs (t1_tissue, partition_coefficient, t1_blood, labeling_efficiency, pld, label_duration) and difference_method now actually flow into quantification (previously collected but ignored). ASLPipeline auto-constructs m0/difference/quant from the validated config. Gate: 845 passed, 0 failed (core incl. GPU); partition_coefficient doubling doubles CBF; multi_pld yields CBF+ATT; nested ASL YAML round-trips. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply the registry-driven config pattern to IVIM (osipy/ivim/config.py): the fitting strategy (segmented/full/bayesian, per-method knobs incl. bayesian-only prior_scale/noise_std) and the signal model (biexponential/simplified) become generated discriminated unions in IVIMPipelineYAML. The simplified model is now selectable (and a latent dS/df Jacobian bug in it, previously dead, is fixed). initial_guess is now a real IVIMFitParams field threaded into BoundIVIMModel (previously set on the config but never forwarded). IVIMPipeline auto-constructs fitter + model from the validated config. Gate: 874 passed, 0 failed (core incl. GPU); nested IVIM YAML round-trips; bayesian surfaces prior_scale, segmented rejects it; initial_guess seeds the fit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reflect the new nested YAML (model/t1_mapping_method/concentration/population_aif for DCE; m0/difference/quantification for ASL; fitting/model for IVIM; deconvolution for DSC). All 8 real-data CLI pipelines pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert all YAML pipeline-config examples (tutorials, how-tos, architecture) to the new nested registry-driven schema; document newly CLI-selectable capabilities (DCE nonlinear-VFA + linear-concentration; ASL multi-PLD/ATT mode; IVIM simplified model + per-method bayesian knobs + initial_guess; DSC per-method deconvolution params). Add docs/explanation/configuration.md describing the registry x schema config generation. Fix docs/gen_config_docs.py (it imported removed flat config classes and broke mkdocs build) to render the discriminated unions. mkdocs build is clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a description to SPGRConcentrationConfig.method so the generated YAML template shows 'spgr | linear' for the concentration block. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove unused internals that are NOT config toggles / public capabilities: - convolution low-level numerical API (conv/uconv/matrix/deconv + registry); keep only the production-used convolve_aif + expconv - shadowed duplicate DSCAcquisitionParams in dsc/concentration (live one is osipy.common.types.DSCAcquisitionParams) - dead custom exceptions MetadataError / osipy ValidationError (never raised; the cli/config ValidationError is pydantic's) Kept (NOT dead): get_gpu_memory_info (used by BatchProcessor in backend/batch.py), _reset_gpu_cache (test utility), all list_*() registry helpers, legacy SVD classes, register_aif, types.FittingMethod. Gate: 795 passed, 0 failed (core incl. GPU); 8/8 real-data CLI pipelines. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
architecture.md no longer lists fft_convolve / the @register_convolution registry row (removed in the internal cleanup); convolution example shows the two surviving functions (convolve_aif, expconv). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The CLI template generator now derives the 'A | B | C' choice annotations from the schema instead of hand-written description strings: - discriminated-union discriminator lines (method/model/mode/name) list their union members' literals (i.e. the registry keys) - Literal[...] fields list their allowed values Convert the remaining flat str enum fields (data.format, output.format, logging.level, DCE fitter/aif_source, ASL labeling_scheme/label_control_order) to Literal so they validate + autogenerate too; drop the now-redundant hand-written validators and '| ' option strings from descriptions. New @register'd method => its choice appears in --dump-defaults with no edits. Gate: 795 passed, 0 failed (core incl. GPU); 8/8 real-data CLI; mkdocs clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
scripts/verify_local_pipelines.sh hard-codes machine-specific data paths and is a local-only helper; untrack it and add to .gitignore (kept in the working tree). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the 'problem it solves' section and condense; keep the mechanics (MethodConfig, discriminator, registry x schema generation, validate+build), a brief add-a-method note, and the per-modality table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- ruff-format (pre-commit pinned ruff) reformats a test array that the editor formatter left expanded - remove the stale root asl_config.yaml example (flat pre-rollout schema, no longer valid; was already deleted in the working tree) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The option/description comment separator used an em-dash (U+2014); on Windows the dump_defaults -> write_text -> load_config round-trip wrote it as cp1252 0x97 and failed to re-read as UTF-8 (UnicodeDecodeError). Use an ASCII ' - ' separator and assert templates are ASCII in test_dump_defaults_are_valid so it can't regress. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
No description provided.