Allow holding parameters constant in calibrator#226
Conversation
Add an optional `calibrate` field to `calibration_parameters` that lists the parameter names to calibrate (e.g., `["R_poiseuille"]`). Parameters not listed are held constant at their input-file values; the LM optimizer now takes an active-parameter index list and runs the normal equations on the reduced sub-Jacobian, leaving inactive entries of alpha untouched. When the field is absent, all parameters are calibrated, preserving legacy behavior. Adds a regression test (`test_steady_flow_calibration_R_only`) that recovers R=100 from a non-ground-truth initial guess while keeping C, L, and stenosis_coefficient pinned to their ground-truth values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #226 +/- ##
==========================================
+ Coverage 78.63% 79.19% +0.55%
==========================================
Files 70 70
Lines 2790 2802 +12
Branches 318 322 +4
==========================================
+ Hits 2194 2219 +25
+ Misses 596 583 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Replaces the synthetic steady-flow R-only test with one based on the existing VMR calibration fixtures (0104_0001). The test starts from the calibrated reference (so C, L and stenosis_coefficient are at the ground truth), zeros every R_poiseuille in vessels and junctions, and asks the calibrator to recover R only. The reference values are recovered to machine precision and the held-constant parameters are unchanged. Also fixes the clang-format violations CI flagged on the previous commit and removes the steady-flow JSON case (the dirgraph test auto-discovers JSONs in tests/cases/ and would have required a generated reference dot file). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each vessel and BloodVesselJunction can now carry its own optional ``calibrate`` list naming the parameters to optimize. Resolution order for each block: block-level ``calibrate`` > ``calibration_parameters.calibrate`` > calibrate everything (legacy). An explicit empty list at any level means "calibrate nothing for that scope". The active-parameter id list passed to LM is built per block from the resolved set, so the optimizer still operates on a global reduced subspace. Adds two regression tests on the smallest VMR case: - ``test_calibration_vmr_R_only_per_block`` — sets the new field on every vessel and junction with no global default. - ``test_calibration_vmr_block_overrides_global`` — sets a wrong global default (``["C"]``) and overrides each block to recover R; verifies the override actually wins. The previous global-only test is renamed to ``test_calibration_vmr_R_only_global``. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes hardcoded knowledge of BloodVessel/BloodVesselJunction parameter
names ("R_poiseuille", "C", "L", "stenosis_coefficient") from
calibrate.cpp. Each block now exposes its parameter names via the
existing ``Block::input_params`` field, and ``Block::input_params_list``
distinguishes scalar layout (one slot per name) from list layout
(``input_params.size() * stride`` slots, name-major). Three small
helpers (``num_param_slots``, ``register_active``, ``init_alpha_for_block``,
``write_alpha_for_block``) walk the parameters generically; alpha
init, active-id selection, and JSON output writing all use the block's
own metadata. New blocks that implement ``update_gradient`` and
populate ``input_params`` will be calibrated without further changes
to calibrate.cpp.
The legacy ``calibrate_stenosis_coefficient: false`` flag is preserved
as an additional filter that excludes ``stenosis_coefficient`` from
the active set; behavior is observably identical (the slot is always
allocated but not optimized).
All seven calibrator tests pass; the existing forward-solver tests
are unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the in-test config builders in tests/test_calibrator.py with three checked-in fixtures derived from the existing 0104_0001 VMR data: tests/cases/vmr/input/0104_0001_calibrate_R_only_global.json tests/cases/vmr/input/0104_0001_calibrate_R_only_per_block.json tests/cases/vmr/input/0104_0001_calibrate_R_only_block_overrides.json Each starts from the calibrated reference (so C, L and stenosis_coefficient are at the ground truth) with every R_poiseuille zeroed; the test parametrizes the same recovery assertion across all three. The Python helpers that built these configs at runtime are gone. Adds docs/pages/calibrator.md describing the new ``calibrate`` field, its global vs per-block resolution order, and pointing readers at the new fixtures as worked examples. Linked from docs/pages/main.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes the global ``calibration_parameters.calibrate`` option. The ``calibrate`` field is now exclusively a per-block setting on vessels and multi-outlet junctions. If no block sets it, the legacy "calibrate every parameter" behavior still applies. Consolidates the three R-only fixtures into a single ``tests/cases/vmr/input/0104_0001_calibrate_R_only.json`` (the former per-block variant); the global and block-overrides fixtures, which exercised paths that no longer exist, are deleted. The parametrized ``test_calibration_R_only`` collapses to one assertion. Updates ``docs/pages/calibrator.md`` to document only the per-block form, drops the resolution-precedence section, and points readers at the single new fixture as a worked example. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes the legacy "no calibrate field => calibrate everything" fallback. Each block must now declare a ``calibrate`` list naming the parameters to optimize; a missing or empty list means every parameter of that block is held at its input value. Drops the ``calibrate_stenosis_coefficient`` flag from ``calibration_parameters``: with explicit per-block selection it is redundant. Removed from the reader in calibrate.cpp and from the fixtures that still carried it. Updates every checked-in calibrator fixture to the new schema: * ``tests/cases/steadyFlow_calibration.json`` * ``tests/cases/vmr/input/0080_0001_calibrate_from_0d.json`` * ``tests/cases/vmr/input/0104_0001_calibrate_from_0d.json`` * ``tests/cases/vmr/input/0140_2001_calibrate_from_0d.json`` Each gets ``calibrate: ["R_poiseuille", "C", "L", "stenosis_coefficient"]`` on every vessel and ``calibrate: ["R_poiseuille", "L", "stenosis_coefficient"]`` on every multi-outlet junction so the existing test expectations (``test_steady_flow_calibration``, ``test_calibration_vmr``) still hold. Updates ``docs/pages/calibrator.md`` to document the mandatory form and shows the explicit lists needed to opt every parameter into calibration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes the BloodVessel-specific ``set_capacitance_to_zero`` switch from ``calibration_parameters`` along with the name-based output post-processing (``std::max(C, 0)``, ``std::max(L, 0)``). With explicit per-block ``calibrate`` selection, these legacy clamps are unnecessary and the calibrator no longer special-cases parameter names by string. Folds the new R-only test case into the existing ``test_calibration_vmr`` parametrize and switches the comparison to the proper ``reference/`` files (which were always intended). Fixes the latent bug where ``np.isclose`` was called without ``assert`` and the comparison referenced ``input/`` instead of ``reference/`` - ``calibrator(input)`` matches every reference to ~1e-13, so the existing three VMR cases now actually verify their output. The R-only case is just one more entry with the same comparison code. Removes the local ``docs/pages/calibrator.md`` and link from ``main.md``: the canonical user guide is in the SimVascular documentation site repo and will be updated there. Drops ``set_capacitance_to_zero`` from every checked-in fixture. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Python formatter that re-emitted the calibrator fixtures was ignoring the parent dict key's prefix when deciding whether an array fits inline. That left one over-80 line in ``0080_0001_calibrate_from_0d.json`` (``"outlet_vessels": [4, 37, 49, 59, 70, 73, 91, 94, 100, 137, 156, 168, 186, 198],``, 86 chars) which prettier flagged in CI. Re-emitted all calibrator fixtures with the fixed formatter; round-trip on the master VMR files matches byte-for-byte and no line in any fixture is over 80 chars. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Hi Martin, I tested the branch using "steadyFlow_calibration.json" and noticed a few things that may be worth double-checking.
I noticed similar warnings in multiple header files. The code compiles successfully, however, it may be worth cleaning up by consistently marking overridden virtual methods with the "override" specifier. Let me know what you think, thank you! |
Add the missing `override` specifier across all Block-derived headers, silencing the -Winconsistent-missing-override warnings raised in review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks, @federicaninno! I fixed the compiler warnings. The other two are identifiability problems: |
|
Thanks for the clarification! In that case, would it make sense for only the identifiable parameters (i.e., "R_poiseuille" and "stenosis_coefficient" in this steady-state case) to be listed under "calibrate"? Alternatively, should we add an unsteady test case where all parameters are identifiable and can be calibrated? I also noticed another behavior that I wanted to check with you. If I modify "R_poiseuille" and calibrate only that parameter, the calibrated value converges back to the original value in the JSON file. |
|
I updated the steady test case to only calibrate We do have unsteady test cases, |
|
That all makes sense! And yes, it is negative but very small. Good to be merged for me! |
|
@federicaninno, thank you! |
…ation Replaces the removed `calibrate_stenosis_coefficient` and `set_capacitance_to_zero` toggles with the new per-block `calibrate` field, and adds a section describing how to declare which parameters the calibrator should optimize. Tracks SimVascular/svZeroDSolver#226.
2 participants
Current situation
The calibrator currently works only for
BloodVesselandBloodVesselJunctionelements, which were hardcoded incalibrate.cpp. By default, it calibrates all parameters in all blocks. This is part of restructuring the calibrator #153. This also closes #221.Release Notes
With this PR, the user can choose, for each block, which parameters should be calibrated. Non-calibrated parameters remain at their input-file values. This enables support for future calibrated blocks (if they implement a Jacobian matrix). I also removed the obsolete (and untested)
calibrate_stenosis_coefficientandset_capacitance_to_zerocalibration options.Documentation
I'm still not a fan of having the documentation in a separate repository. Makes me forget to update the documentation and open another pull request. Oh well: SimVascular/simvascular.github.io#92
Testing
Explicitly added the calibrated parameters to existing test cases and added a new test case that only calibrates the resistances. Still works.
Code of Conduct & Contributing Guidelines