Throw on malformed outcome rates row - #27
Merged
mtcorread merged 5 commits intoSep 13, 2026
Merged
Conversation
added 2 commits
September 8, 2026 14:56
validateOutcomeRowSums already detected rows that do not sum to 1, and only printed to stderr. It fired on 5 of the 40 rows of the covid19 table, worst sum 1.326, for the life of that file, and the warning was read and ignored. The outcome columns are a probability distribution over trajectories, so a row that does not sum to 1 is not a table computeTrajectoryRates can sample from: the weights are renormalised only above 1.0001 and negatives are never clamped, so the overshoot silently redistributes probability along the trajectory walk. Also rejects negative rates, as a second net rather than the first one - a negative can hide inside a row that still sums to 1. Replaces data/infection_outcome_rates_covid19.csv with the rebuilt table this now requires. It was the file tripping the check, and configs/config_2021 reads it, so test_simulator and the two MPI reproducibility tests failed against the old copy the moment the warning became an error. The rebuild itself belongs to my_june, which generates this file from a sourced evidence databank and keeps this copy as its mirror; see its plans/rebuild-covid-outcome-rates.md. It is 42 rows against 40, keyed on residence venue type and XLGU as well as age and sex, every row summing to 1 with nothing negative. All 52 tests pass.
gavdoubleu
force-pushed
the
fix/outcome-rates-row-sum-throws
branch
from
September 8, 2026 14:15
0ae7dfb to
3c169a3
Compare
Collaborator
|
Thanks Gavin. The loader change is right, it's small, and it's what the title says, so I'd like to merge it on its own. Could you split this PR in two? 1. Throw on a malformed row (this PR). Keep the 18 of the 40 rows fail the new check (row sums up to 1.326), and 7 of them carry a negative 2. The replacement COVID table (a separate PR). It should come with what it's built from: That second PR also uses |
Contributor
Author
|
Hi Martha,
I'm not really sure how to do suggestion 1 without just rebuilding the data, please advise.
Because Frank's never replied about how the original outcome rates were acquired, I just rebuilt the table completely from scratch (what you're saying PR 2 is).
I do think 1 is fine, but it still requires a totally new outcome-rates table --- not sure what you mean by 'make valid' some rows without touching others and keeping broadly the same table?
With regards to 2, I can add the scripts, evidence, etc if you like but I'll say that
1.
we didn't have these before, we just put the table in data with nothing to back it up
2.
I think really belongs on the data drive - which the scripts/get_data.sh downloads, not in JUNE2 as we don't have anything else committed in JUNE2 to do with how data is gathered or evidenced.
3.
It is already recorded on my_june repo, so this evidence is backed up somewhere (although my_june repo is quite messy as it's really just for me).
Do you actually want the plans and scripts used to build my replacement outcome table committed in JUNE, or is that just what claude has suggested?
Maybe I should just remove the comments from the example config file so nobody gets confused?
And yes PR #27 should land after #25, hence the order in which I did them.
All the best,
Gavin
…________________________________
From: Martha Correa-Delval ***@***.***>
Sent: 13 September 2026 21:32
To: IDAS-Durham/JUNE2 ***@***.***>
Cc: Gavin W ***@***.***>; Author ***@***.***>
Subject: Re: [IDAS-Durham/JUNE2] Throw on malformed outcome rates row (PR #27)
[https://avatars.githubusercontent.com/u/165719743?s=20&v=4]mtcorread left a comment (IDAS-Durham/JUNE2#27)<#27 (comment)>
Thanks Gavin. The loader change is right, it's small, and it's what the title says, so I'd like to merge it on its own. Could you split this PR in two?
1. Throw on a malformed row (this PR). Keep the disease_loader.cpp change here. It can't land by itself yet, though: against the COVID table currently on main, config_2021 no longer loads. I built main plus only this loader change and ran it:
Outcome rates row 20 sums to 1.011326, expected 1.0.
18 of the 40 rows fail the new check (row sums up to 1.326), and 7 of them carry a negative severe. So this half needs those rows in the existing table made valid, however you think is right for the data, without bringing in the whole replacement.
2. The replacement COVID table (a separate PR). It should come with what it's built from: scripts/build_outcome_rates.py and data/infection_outcome_rates_evidence/. The CSV header cites both, plus plans/rebuild-covid-outcome-rates.md and CONTEXT.md, and none of them exist on any branch. As it stands the table can't be regenerated or checked. Please drop the plans/CONTEXT references from the header either way.
That second PR also uses filter.geo_unit.XLGU, which only exists once #25<#25> is in, so it has to land after #25<#25>.
—
Reply to this email directly, view it on GitHub<#27?email_source=notifications&email_token=BVNZ65QZM3WF2JKUO55IAYT5O377DA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNRVGU4TGNZRGYZ2M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5655937163>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BVNZ65WW77DOSVPPHP5DAJD5O377DAVCNFSNUABGKJSXA33TNF2G64TZHMYTEMZZGE3TOOBXGY5US43TOVSTWNJSGY4TKNZVGIYTNILWAI>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/BVNZ65VAIVBKGLUCSN6WKCT5O377DA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNRVGU4TGNZRGYZ2M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/BVNZ65XUJYLPYKKWAUXQ3WL5O377DA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNRVGU4TGNZRGYZ2M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Collaborator
|
Definitely Claude went rogue there... I was like 'check PR 27', left to the bathroom and come back to him commenting on my behalf 💀. No, I think we don't need to add those back or split the PR. Do not worry, also I didn't expect you to be checking this at this time! Go to bed 😝 |
Contributor
Author
|
Hey you are working at this time too ! 😋
Night!
…________________________________
From: Martha Correa-Delval ***@***.***>
Sent: 13 September 2026 22:06
To: IDAS-Durham/JUNE2 ***@***.***>
Cc: Gavin W ***@***.***>; Author ***@***.***>
Subject: Re: [IDAS-Durham/JUNE2] Throw on malformed outcome rates row (PR #27)
[https://avatars.githubusercontent.com/u/165719743?s=20&v=4]mtcorread left a comment (IDAS-Durham/JUNE2#27)<#27 (comment)>
Definitely Claude went rogue there... I was like 'check PR 27', left to the bathroom and come back to him commenting on my behalf 💀. No, I think we don't need to add those back or split the PR. Do not worry, also I didn't expect you to be checking this at this time! Go to bed 😝
—
Reply to this email directly, view it on GitHub<#27?email_source=notifications&email_token=BVNZ65RR7SVQNOTUMPG5U735O4D43A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNRVGYYTGOJVGY4KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5656139568>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BVNZ65S5OJQPRLODNTIRFU35O4D43AVCNFSNUABGKJSXA33TNF2G64TZHMYTEMZZGE3TOOBXGY5US43TOVSTWNJSGY4TKNZVGIYTNILWAI>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/BVNZ65RASTO7CEDR2ELW3235O4D43A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNRVGYYTGOJVGY4KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/BVNZ65XW4SIHPJK7OMYATB35O4D43A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNRVGYYTGOJVGY4KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Collaborator
|
I didn't work on Friday so I was gonna make up for it today :P |
The header named a generator script, a plans document, a glossary and an evidence directory. None of them are in JUNE2; the table's provenance lives with the data, not here, so the references only pointed nowhere.
mtcorread
added a commit
that referenced
this pull request
Sep 13, 2026
test_simulator built small worlds by hand and then loaded configs/config_2021/disease.yaml and infection_seeds.yaml, so it broke whenever the production configs grew something those worlds couldn't answer. #27's outcome table filters on XLGU, which neither test world declares, and the load-time check throws. The test now loads tests/configs/simulator_disease.yaml, which picks trajectories by fixed probability and has no outcome table, and tests/configs/simulator_infection_seeds.yaml, which seeds on the test's own start date. The old seed file was dated a month after the run, so the smoke test never infected anyone. With its own seed file it infects 5 of its 20 people.
mtcorread
approved these changes
Sep 13, 2026
mtcorread
added a commit
to gavdoubleu/JUNE2
that referenced
this pull request
Sep 13, 2026
This brings in IDAS-Durham#30 and IDAS-Durham#27 from main without conflicts. IDAS-Durham#30 only changes test_simulator and adds its own test configs, and IDAS-Durham#27 only changes the outcome-rates loader and the COVID table, and this branch touches none of those files.
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
2 participants
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
infection_outcome_rates_covid19.csvto pass the new checkReview
Code-reviewed at medium effort: no correctness bugs, no dropped invariants.