Fix silent contact-matrix/transmission-mode misattribution#20
Open
gavdoubleu wants to merge 21 commits into
Open
Fix silent contact-matrix/transmission-mode misattribution#20gavdoubleu wants to merge 21 commits into
gavdoubleu wants to merge 21 commits into
Conversation
added 18 commits
May 28, 2026 15:00
Reproducible conda environment for building and running JUNE2. Includes cxx-compiler (GCC on Linux), cmake, hdf5, yaml-cpp, openmpi, Python test deps (h5py, numpy), and optional deps metis and gperftools.
default_contacts is a hardcoded scalar fallback that ADR-0006 deletes,
since it silently masks a parsing bug where modes:-format
default_contacts_matrix configs resolve empty. Ahead of that fix, ~15
call sites across 11 test files set cm.default_contacts directly as a
fixture knob for a distinguishable contact-rate canary; each is
rewritten to build a one-bin ContactMatrix (bins={"all"},
contacts={{X}}) and assign it to cm.default_matrix instead, which
getMatrix() already consults before falling through to the scalar, so
runtime behaviour is unchanged. Also drops test_config_loading.cpp's
SUBCASE asserting the scalar loads from YAML, since that field is
going away; the YAML fixtures' now-unused default_contacts: keys are
left alone for the follow-up loader change to clean up.
All 11 rewritten test binaries pass (test_mpi_transmission_modes
verified via mpirun -np 2).
default_contacts_matrix in contact_matrices.yaml was parsed with the flat-only parser even though every production config writes it in modes: format, so it silently resolved empty and every lookup fell through to a hardcoded default_contacts scalar (2.0) with no relation to the user's configured beta. Step 1 of the fix (loader/runtime changes to actually populate and require the new fields land separately): - ContactMatrixConfig gains default_mode_matrices (mode_name -> ContactMatrix) and a resolved default_mode_matrices_by_id lookup, mirroring the existing per-venue mode_matrices/mode_matrices_by_id pattern. getMatrix(venue_type_id, mode_index) now falls back to the per-mode default before the flat default_matrix. - resolve() now bin-resolves default_matrix and each entry of default_mode_matrices (male/female bin, subset-type bin, age-to-bin table) - default_matrix previously skipped this step entirely, so any lookup that reached it saw unresolved bin fields. - Deleted ContactMatrixConfig::default_contacts. Its two remaining call sites are stubbed pending the next steps: config_loader.cpp no longer reads the now-deleted YAML key, and InteractionManager::lookupContactsForBinPair returns 0.0 with a TODO where it used to return the scalar (load-time validation coming next will make that branch unreachable in practice). Full build and test suite pass (42/43; the one failure is a pre-existing missing-world-file issue unrelated to this change).
default_contacts_matrix was always parsed flat, so every production config (which writes it in modes: form) silently resolved to an empty matrix and fell through to the now-deleted default_contacts scalar. Rewrite ConfigLoader::loadContactMatrices to require the key (throws naming the filename if absent) and dispatch on modes: presence, mirroring parseContactMatricesList's per-venue handling: modes: form parses each mode into default_mode_matrices and validates coverage against config.mode_names, throwing on any disease mode left uncovered; flat form parses into default_matrix as before. Drop the now-dead default_contacts: scalar lines from contact_matrices.yaml and romantic_regression.yaml fixtures; the latter had no default_contacts_matrix at all, so add a flat one (mirroring romantic_encounter's shape) since the key is now mandatory. Add SUBCASEs and fixtures exercising modes-form loading, the missing-key throw, and the missing-mode-in-per-mode-default throw.
By this point mode_matrix/fallback_matrix already route through ContactMatrixConfig::getMatrix's default-matrix fallback chain (now guaranteed non-empty since default_contacts_matrix is required, see 2aa6dd3/9bed681), so both being null/out-of-bounds means mismatched bins - a real bug to surface, not a made-up contact rate to hide it. Full ctest run unaffected: 42/43 pass, same pre-existing unrelated test_mpi_full_reproducibility failure as before.
getVirtualMatrix(encounter_type_id, mode_index) never consulted default_contacts_matrix, unlike getMatrix's venue-path chain -- an encounter type with no matching virtual_contact_matrix entry (missing config, typo) resolved to a null matrix and crashed lookupContactsForBinPair at first use, instead of failing loud at config-load time or falling back sensibly like venues do. Extracted applyDefaultChain() as the shared tail (per-mode default, then flat default_matrix) used by both getMatrix and getVirtualMatrix, so virtual encounters now fall through to the same default_contacts_matrix a venue would -- no separate virtual-specific default. Added a regression test covering an unconfigured encounter type, and corrected the now-stale invariant comment on lookupContactsForBinPair.
…esolve Four near-identical 6-line blocks (male_bin/female_bin/bin_by_subset_type/ age-bin resolution) had built up across per-venue matrices, per-venue mode matrices, default_matrix and default_mode_matrices - the latter two added this branch to satisfy ADR 0006's fail-loud requirement, copy-pasting the existing per-venue pattern rather than reusing it. A fifth, already- deduplicated copy (resolve_matrix_bins lambda + bin_resolved guard) existed further down for virtual-encounter matrix aliasing. Hoist resolve_matrix_bins above all five use sites and call it everywhere; delete the now-redundant second declaration. The dedup guard is a no-op for the four venue/mode/default sites since each matrix there is touched once, so behaviour is unchanged. Net -35 lines, one place to edit bin-resolution semantics instead of five.
default_contacts was replaced by default_contacts_matrix (2aa6dd3) and the loader no longer reads it (parseContactMatrixScalars deleted). Left in place it silently misleads: a researcher editing default_contacts next to default_contacts_matrix would expect it to affect the sim, but it has zero effect.
Files stay on disk but git no longer tracks them.
A config supplying default_contacts_matrix.modes but no contact_matrices venue modes: block left ContactMatrixConfig::mode_names empty, so resolve() skipped building default_mode_matrices_by_id entirely (gated on !mode_names.empty()). The per-mode default was silently discarded at load, surfacing later as a runtime std::runtime_error from InteractionManager::lookupContactsForBinPair - exactly what ADR 0006's load-time validation was meant to prevent. Add ContactMatrixConfig::finalizeDefaultModeMatrices(world, disease_mode_names), which rebuilds default_mode_matrices_by_id keyed to an explicit disease mode-name list instead of mode_names, and throws if a disease mode has neither a per-mode default entry nor a flat default_matrix. Wired into Simulator::Simulator right after disease_ loads, the first point both Disease and ContactMatrixConfig co-exist. Required widening Simulator::config_ from const Config& to Config&. A disease mode absent from mode_names/per-venue matrices is fine (falls back to default); absent from the default itself is now a fatal, loud error rather than a silent nullptr. Also fixes two test fixtures (test_simulator.cpp) that relied on the old bug's permissiveness by never configuring a default matrix at all. Docs: corrected CONTEXT.md's Transmission Mode/Default Contact Matrix entries, added ADR 0007 recording the Simulator-constructor placement decision (and rejected alternative of reworking resolve()'s signature). The separate, deferred order/index-alignment gap between mode_names and transmission_params.modes (positional lookup, unvalidated) is flagged as future work only, not fixed here.
…deMatrices d188e3c added finalizeDefaultModeMatrices() and copy-pasted the male_bin/female_bin/bin_by_subset_type/age_to_bin logic into a new free function rather than reusing resolve()'s resolve_matrix_bins lambda - undoing the dedup 493bbbb had just done, and leaving a comment claiming the logic was "shared" when resolve() still ran its own copy. resolve_matrix_bins now delegates to the free function; resolve()'s local resolveAgeToBin lambda is gone. One implementation, both callers.
disease.yaml's transmission_params.modes and contact_matrices.yaml's per-venue modes: blocks are independently-ordered lists. getMatrix/ getVirtualMatrix indexed contact matrices by the raw disease mode index, silently misattributing a matrix to the wrong mode whenever the two files declared modes in different orders (e.g. fomite applied to droplet transmission). A prior fix only corrected the default-fallback path; this fixes the primary per-venue/per-encounter lookup. Add ContactMatrixConfig::finalizeDiseaseModeAlignment, building a name-matched disease-mode-index -> contact-matrix-mode-index permutation vector once at Simulator init (avoids per-lookup string comparison). getMatrix/getVirtualMatrix translate through it before indexing; a disease mode absent from contact_matrices.yaml falls through to the existing default chain, and an orphaned contact_matrices.yaml mode warns rather than erroring (configs may be shared across diseases with differing mode sets). Add regression test with reversed mode order between the two config files, plus fixture tests/configs/contact_matrices_reversed_modes.yaml. Update CONTEXT.md's Transmission Mode entry and a stale comment in disease.h that assumed index-based matching.
Follow-up to bc18375's finalizeDiseaseModeAlignment fix; review surfaced two ways the same silent-misattribution failure mode could recur. 1. Duplicate/missing disease mode names aliased silently: linear-search name matching broke on first match, so two transmission_params.modes entries sharing a name (or both omitting one, defaulting to "default") collided and the second mode's matrix silently aliased onto the first's. finalizeDiseaseModeAlignment now throws on duplicate disease mode names instead of aliasing. 2. Ambiguous "not finalized" sentinel: translateModeIndex used an empty mode_index_translation_ to mean "finalize not yet called, fall back to raw positional indexing". But finalizeDiseaseModeAlignment produces the same empty vector when called with zero disease modes, so that state was indistinguishable from "never finalized" and would incorrectly revert to unsafe raw positional lookups. Added disease_mode_alignment_finalized_ as an explicit sentinel, decoupling "finalized" from vector emptiness. Add regression tests for both cases; update CONTEXT.md's Transmission Mode glossary entry for the uniqueness requirement.
added 3 commits
July 23, 2026 10:32
Fix Lint Code Base CI failure on PR IDAS-Durham#20; whitespace-only, matches the CI-pinned clang-format version rather than the locally-installed one.
Field name was misleading: it scales a transmission mode's contribution to force-of-infection (a per-route, per-mode constant), not an individual's biological susceptibility (which is Person::getSusceptibility, driven by waning immunity/vaccine efficacy). Renamed the TransmissionMode struct member, disease_loader.cpp YAML key, all disease/contact-matrix config files, tests, and USER_GUIDE.md docs to avoid confusing the two concepts.
Longer identifier pushed two lines over the column limit; rewraps them to satisfy the CI clang-format check.
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.
Title: Fix silent contact-matrix/transmission-mode misattribution (default-matrix path + per-mode path)
Original bug
contact_matrices.yaml'sdefault_contacts_matrixis meant to be the fallbackcontact matrix used whenever a (venue type, transmission mode) pair has no
dedicated entry. Every production config writes this key in
modes:form(per-mode matrices), but the loader only ever parsed it as a flat single
matrix. Result:
default_contacts_matrixsilently resolved to an emptymatrix on every real config, and every lookup that needed it fell through to
a hardcoded scalar,
ContactMatrixConfig::default_contacts = 2.0, with noerror, no warning, and no relation to whatever beta/contact rate the user had
actually configured. Simulated infection numbers for any venue/mode without
an explicit matrix were silently wrong, and there was no way to tell from
config or logs that this was happening.
There was a second, more subtle layer to this: even after fixing the parser,
ContactMatrixConfig::resolve()only built the per-mode default lookup table(
default_mode_matrices_by_id) whenmode_nameswas non-empty. A configthat supplies
default_contacts_matrix.modesbut has no venue-levelcontact_matrices: ... modes:block leavesmode_namesempty, so the tablewas never built and the per-mode default stayed unreachable — surfacing
later as a runtime
std::runtime_errorinstead of the load-time failure ADR0006 was meant to guarantee.
A third, independent bug affected the primary (non-default) per-mode lookup:
disease.yaml'stransmission_params.modesandcontact_matrices.yaml'sper-venue
modes:blocks are two independently-ordered lists.getMatrix/getVirtualMatrixindexed contact matrices by the raw disease-mode indexinto
contact_matrices.yaml's list, silently misattributing a matrixwhenever the two files declared modes in different orders (e.g. a
fomitematrix applied to
droplettransmission).Fix, in order
default_contacts_matrixin both flat andmodes:form (mirroring the existing per-venue parser), bin-resolve itlike every other matrix, and delete the
default_contactsscalarfallback entirely. The key is now mandatory: a venue/mode with no matrix
anywhere is a load-time error, not a silent stand-in value (ADR 0006).
lookupContactsForBinPairnow throws instead of returning0.0when a matrix is still missing at that point — by then the defaultchain guarantees non-null, so hitting this branch means a real bug
(mismatched bins), not a config gap to paper over.
getVirtualMatrix(virtual encounters) never consulteddefault_contacts_matrixat all, unlike the venue path — an encountertype with no dedicated matrix crashed instead of falling back. Extracted
the shared default-fallback tail (
applyDefaultChain) so venues andvirtual encounters go through the same fallback logic.
mode_names-empty gap described above: addedContactMatrixConfig::finalizeDefaultModeMatrices(world, disease_mode_names), called fromSimulator::Simulatorright afterdisease_loads (the first point both the disease's canonical mode listand
ContactMatrixConfigco-exist), which rebuilds the per-mode defaulttable keyed to the disease's actual modes and throws if any disease mode
has neither a per-mode default nor a flat fallback (ADR 0007).
duplicated from
resolve().default_contactskey fromtracked config YAMLs; untracked
config_plague_fitting/config_plague_fitting_short(large generated fixtures that shouldn't beversion-controlled).
mismatch described above. Added
ContactMatrixConfig::finalizeDiseaseModeAlignment, building aname-matched disease-mode-index → contact-matrix-mode-index permutation
vector (
mode_index_translation_) once atSimulatorconstruction,right after
finalizeDefaultModeMatrices.getMatrix/getVirtualMatrixtranslate through it via a new
translateModeIndexhelper beforeindexing. A disease mode absent from
contact_matrices.yamlfallsthrough to the existing default chain; a
contact_matrices.yamlmodeabsent from the disease warns and is ignored (configs may be shared
across diseases with differing mode sets).
mechanism, found by review:
finalizeDiseaseModeAlignment's name-matching used break-on-first-matchwith no uniqueness check, so two
transmission_params.modesentriessharing a name (or both omitting one, defaulting to
"default")collided — the second mode's matrix silently aliased onto the first's,
the same failure class as step 7 fixes, just triggered by an in-file
naming collision rather than cross-file ordering. Now throws on a
duplicate disease mode name instead of aliasing.
translateModeIndexused an emptymode_index_translation_to mean"not yet finalized, fall back to raw positional indexing" — but
finalizing with a genuinely empty disease-mode list produces the same
empty vector, making "finalized, zero modes" indistinguishable from
"never finalized" and reverting to unsafe positional lookups. Added an
explicit
disease_mode_alignment_finalized_flag as the sentinelinstead of inferring it from vector emptiness.
See
docs/adr/0006-default-contact-matrix-fails-loud-no-scalar-fallback.mdand
docs/adr/0007-default-mode-matrix-finalised-in-simulator-constructor.mdfor the design rationale behind steps 1-6. Steps 7-8 are bug fixes to that
existing mechanism, not new architectural decisions, so no new ADR.
Test plan
ctest— fixtures rewritten offContactMatrixConfig::default_contacts;new fixtures cover missing default, missing mode in default, empty
mode-names-with-per-mode-default, reversed disease/contact-matrix mode
order, orphaned contact-matrix mode names, duplicate disease mode names,
and zero-disease-mode finalization
default_contacts_matrix.modesand no venue-levelcontact_matrices: modes:block — confirm it no longer throws at runtimeand no longer silently uses a stale scalar
disease.yamlandcontact_matrices.yamldeclarethe same mode names in different orders — confirm matrices attach to the
correct mode