fix: include exact gestational chart boundaries - #101
Conversation
|
@eatyourpeas this is a question for you really. I'm reviewing as much of the dGC Python/API/Chart/Demo code as possible, bit by bit. Where I find possible inconsistencies I'm flagging them, in case they were not deliberate. This one could have been deliberate - if it was then that's fine we can close the PR and make some comments in the code that explain the decision. |
|
I am not sure i understand this but yes, 25 weeks should be exactly 25 weeks and so on. It is important that the charting allows plotting of values below these gestations as babies these days are born and save at 22 w, so they need to be plottable even if there is no reference data down there to generate and SDS. |
Review: the truncated boundary literals, and a regression the branch introducesThis one took me (and some very experienced growth-calculations colleagues) several passes to fully understand, so this comment deliberately spells the numerics out. The core idea of this PR is right, and it does restore real boundary points - but as written it also silently drops the age-2 and age-4 chart points that PR #102 added. Detail and evidence below. What the recurring decimal actually is
It legitimately appears in three different roles in the codebase, which is why it "turns up more than once":
Because UK90 preterm data and WHO 2006 infant data both contain a 42-week row, this single age is the seam between two reference datasets. That seam is the whole story of this PR. Why the truncation matters: floating-point comparison against the data tablesThe reference data tables ( The truncated literal is a different float from the constant (they differ by ~2e-11). Whether that matters depends on the sign of the error, and here it bites at the lower boundary:
The chart code rounds x-coordinates to 4 d.p. for display, so the rendered points look fine - what goes missing is the first and last data points of each preterm series. Concretely, on
So: the fixes themselves are real and clinically meaningful - the charts genuinely were missing their outermost preterm boundaries. The regression: age-2 and age-4 points disappearThis PR removes
Without the clamp,
The same failure is reproducible directly: Why the branch's own tests do not catch it
Suggested resolution
One smaller question: the new BMI test expects a point at None of this changes the fact that the 23/25/42-week restoration is correct and valuable - it just needs to land on top of the age-two fix rather than instead of it. |
|
Updated this branch on top of current I kept broader decimal-grid generation out of this PR: some lists intentionally preserve source rounding, so that deserves a separate, list-preserving review rather than being mixed into this boundary fix. |
Clinical/reference-data decision
Following @eatyourpeas's response, the implementation now makes the distinction explicit:
Implementation
Clinical safety
This does not extrapolate reference data or broaden the range in which SDS can be calculated. A 22-week observation remains available in
plottable_datawith its exact corrected x-coordinate and raw y-value, while SDS and centile remainNonewith the below-reference explanation.Testing
s/lint --running