Reuse forward-solver assembly for calibrator residual#227
Draft
Reuse forward-solver assembly for calibrator residual#227
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>
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>
Until now the per-block ``update_gradient`` re-implemented two things: the parameter Jacobian columns and the residual itself, with the latter duplicating the (E, F, c) algebra that the forward solver already owns. This commit splits the two concerns: * ``Block::update_gradient`` is contracted to populate only the parameter Jacobian columns; the residual argument is removed from the signature and the implementations of BloodVessel, BloodVesselCRL and BloodVesselJunction lose their residual-formula branch. Junction's override (which only wrote the residual and had no parameters) is dropped entirely; the new base-class default is a no-op so blocks without parameters need no override. * The Levenberg-Marquardt optimizer now owns a SparseSystem and, at each iteration, syncs alpha into the model's parameter storage, calls ``Model::update_constant`` once and ``Model::update_solution`` per observation, and reads the residual chunk straight off the assembly (``r_i = c + E*dy + F*y``). This is the same residual the forward solver computes (up to the sign that cancels in the LM normal equations). Two practical wrinkles addressed: * Calibrator models contain no boundary-condition blocks, so ``num_vars > num_eqns``. The system is sized with the larger of the two; matrix-vector products run on padded vectors and the residual segment is sliced to the equation count. * ``calibrate.cpp`` now registers each parameter slot through ``Model::add_parameter`` so ``parameter_values`` is sized for the assembly path. The LM loop syncs alpha through ``Model::update_parameter_value``; ``update_time`` is intentionally skipped because it would overwrite the synced values from the Parameter objects. All seven calibrator tests still pass, every solver/io test still passes, and ``make codeformat`` produces no diff. 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 #227 +/- ##
==========================================
+ Coverage 78.63% 79.30% +0.66%
==========================================
Files 70 70
Lines 2790 2803 +13
Branches 318 325 +7
==========================================
+ Hits 2194 2223 +29
+ Misses 596 580 -16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The calibrator residual is computed at observation samples that carry no time stamp, so any block with non-trivial ``update_time`` cannot be calibrated correctly. ``Block`` gains a virtual ``has_time_dependent_assembly()`` (default false); chambers and the time-dependent boundary conditions override it to return true. ``calibrate.cpp`` walks the model after ``finalize`` and throws before reading observations or constructing the optimizer if any such block is present. Restores ``Block::update_gradient``'s default to throw "not implemented" so a parameterized block that forgets to override is caught instead of silently producing a zero Jacobian column. The Levenberg-Marquardt loop skips blocks with no parameters (``global_param_ids.empty()``) so parameterless blocks like NORMAL_JUNCTION don't trip the throw and don't need their own no-op override. Co-Authored-By: Claude Opus 4.7 (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.
Summary
update_gradientinto (a) writing only the parameter-Jacobian columns and (b) computing the residual via the existing forward-solver assembly. The residual formulas are no longer duplicated between solver and calibrator.BloodVessel,BloodVesselCRL,BloodVesselJunctionnow write Jacobian only.Junction's override is dropped (it had no params, only wrote residual). The base-class default is a no-op so blocks without parameters need no override.SparseSystemand, each iteration, syncs alpha intomodel.parameter_values, callsmodel.update_constantonce andmodel.update_solutionper observation, and readsr_i = c + E*dy + F*ystraight off the assembly — same residual the forward solver computes.Why
Until now the per-block
update_gradientre-implemented two things: the Jacobian columns and the residual algebra. The residual algebra is exactly what the solver assembles viaupdate_constant/update_solution, so the duplication meant any change to a block's E/F/c shape had to land in two places. Splitting concerns lets the calibrator inherit residual changes for free.Practical wrinkles
num_vars > num_eqnsis possible. The sharedSparseSystemis sized withmax(num_eqns, num_vars); matrix-vector products run on padded vectors and the residual segment is sliced tonum_eqns.calibrate.cppnow registers each parameter slot viaModel::add_parameter(0.0)soparameter_valuesis sized for the assembly path. The LM loop syncs alpha throughModel::update_parameter_value.update_timeis intentionally skipped — it would overwrite the synced values from the dummyParameterobjects.Stacked on
This branch is layered on top of #226; the Block API change and the LM refactor here depend on the per-block
calibratefield and the block-agnostic helpers introduced there. Marked draft; rebase to master once #226 merges.Test plan
pytest tests/test_calibrator.py— all 5 calibrator tests pass.pytest tests/test_solver.py tests/test_io.py— 49 pass + 2 pre-existing xfail; forward solver unaffected.make codeformatproduces no diff.Diff stat
13 files changed, +90 / -123 lines.
🤖 Generated with Claude Code