fix(params): load flat-model parameter values correctly + save calibrated model (#114)#118
Merged
Merged
Conversation
…ated model (#114) DEFCON 1: uploading a circulatory_autogen *flat* CellML loaded wrong parameter values, so simulations/plots didn't match the file (even for a non-calibrated model). Root cause: a flat model renames constants — params_for_id `aortic_root/C` lives in the model as `parameters/C_aortic_root`, and `global/E_lv_A` as `parameters_global/E_lv_A` — so the direct `vessel/param` qname wasn't found, the slider `initial_value` came back None, and the UI substituted the range MIDPOINT. - params_for_id.py: resolve the initial value by CA's own name mapping when the direct qname is absent — `resolve_model_qname` computes the `param_names_for_gen` name (`param` for the `global` vessel, else `param_vessel`) and looks it up by bare variable name, preferring the flat model's `parameters*` component and only when unambiguous. Direct `vessel/param` still wins first (Lotka-Volterra etc.). Also, per the report, save a calibrated CellML when a run finishes so it can be reloaded and reproduce the calibrated simulation ("make sure the model is saved"): - calibrated_model.py: `calibrated_cellml()` bakes best-fit values into the flat model's initial_value attributes (pure-XML, component-scoped substitution — the exact inverse of the loader), reporting updated/unresolved params. - calibration_runner.py writes `<prefix>_calibrated.cellml` to the output dir on finish (best-effort); calibration.py threads `calibrated_model_path` into the job status; main.py serves it at GET /api/calibration/{job}/calibrated_model. - Frontend: useCalibration exposes `calibratedModelUrl`; CalibrationPanel shows a "Download calibrated model" link when a run is done. So the user can load the calibrated model in a new window and the sliders come back as the best fit. - cuflynx.spec bundles calibrated_model/cellml_meta/params_for_id beside the runners (calibration_runner imports them in the packaged app). Tests: unit (gen-name resolution incl. ambiguity/preference; calibrated-model substitution + round-trip; download route) and an integration test that runs a real GA calibration, then reloads the saved calibrated model and asserts the slider initial values equal the best fit — the exact broken scenario. Backend 350 unit / 378 incl. integration; frontend 297; build clean. 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
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.
Fixes #114 (DEFCON 1): uploading a circulatory_autogen flat CellML loaded
the wrong parameter values, so simulations/plots didn't match the file — for
calibrated and non-calibrated models.
Root cause
A flat model renames its constants. params_for_id
aortic_root/Clives in themodel as
parameters/C_aortic_root;global/E_lv_Aasparameters_global/E_lv_A.The loader only looked up the direct
vessel/paramqname, didn't find it, so theslider
initial_valuecame backNone— and the UI substituted the rangemidpoint. Wrong values in → wrong sim out.
Fix
params_for_id.resolve_model_qname()resolves the value by CA's own name mappingwhen the direct qname is absent: it computes the
param_names_for_genname (paramfor the
globalvessel, elseparam_vessel, exactly perPrimitiveParsers) andlooks it up by bare variable name — preferring the flat model's
parameters*component and only when unambiguous. Direct
vessel/paramstill wins first, sonon-flat models (Lotka-Volterra) are unaffected.
Verified: the 4 params of the 3compartment flat model now resolve to their real
file values (were all
None/midpoint before).Save the calibrated model (the reported test path)
Per "make sure the cellml model is saved when finishing calibration":
calibrated_model.calibrated_cellml()bakes best-fit values into the flatmodel's
initial_valueattributes — pure-XML, component-scoped substitution(the exact inverse of the loader), so the rest of the document is byte-identical.
calibration_runnerwrites<prefix>_calibrated.cellmlon finish; the path isthreaded into the job status and served at
GET /api/calibration/{job}/calibrated_model.done.
Tests
parameters*preference);calibrated-model substitution, "only targeted lines change", round-trip, and the
download route (404s + happy path).
the saved calibrated model has the best-fit values → re-upload it + params_for_id
("new window") → the slider initial values equal the best fit.
yarn buildclean.🤖 Generated with Claude Code