Found while establishing an identical() baseline for #739, on main at
4b1d2633.
The finding
Two runs of unmodified main produce a get_primary_production() result
that is not identical() to itself:
r1 <- get_primary_production() # fresh R session
r2 <- get_primary_production() # another fresh R session, same commit
identical(r1, r2) # FALSE
identical(strip_cb_extracts(r1), strip_cb_extracts(r2)) # TRUE
The published frame — all 6,310,390 rows and 12 columns — is identical().
The difference is entirely in the .cb_extracts attribute, a list of four
tables (fbs_new, fbs_old, cbs_crops, cbs_animals) that
.read_production() attaches and build_primary_production() carries through.
Each of the four holds the same rows with the same values in a different
order:
| table |
rows |
identical() |
identical() after sorting all columns |
fbs_new |
1,218,038 |
FALSE |
TRUE |
fbs_old |
4,062,537 |
FALSE |
TRUE |
cbs_crops |
3,587,646 |
FALSE |
TRUE |
cbs_animals |
1,174,533 |
FALSE |
TRUE |
So it is row order alone, not content. Dropping the attribute or sorting each
table makes the two runs byte-identical.
Why it matters
It is not a wrong number, but it costs reproducibility evidence. Any PR that
wants to assert "the output is unchanged" has to know to strip the attribute
first, and the natural check —
identical(build_primary_production(), baseline)
— returns FALSE on a change that moved nothing. That is exactly the shape of
check the epic has come to rely on (see #566 and the merge notes on #458), and
a control that fails against itself is worse than no control: the next agent
either weakens the assertion to all.equal() or spends an hour looking for a
regression that is not there. It cost about that here.
Where to look
.cb_extracts is set in .read_production() (R/build_production.R) from the
CBS extraction step, before anything else in the pipeline runs, so it is
upstream of any polity or QC code. The likely candidates are an unordered
merge()/dcast()/rbindlist() or a by= grouping whose output order is not
pinned — data.table does not guarantee group order without keyby= or an
explicit setorder().
Suggested fix
Pin the order where the four tables are built (setorderv() on their key
columns, or keyby= instead of by=). It is a diagnostic side-channel, so the
order is not load-bearing for any consumer — the point is only that the object
should be a function of its inputs.
Verification is cheap and does not need this to be reproduced first: build twice
in two fresh sessions and assert identical() on the whole object, attribute
included.
Part of the polity migration epic #458 only incidentally; the defect is in
production I/O.
Found while establishing an
identical()baseline for #739, onmainat4b1d2633.The finding
Two runs of unmodified
mainproduce aget_primary_production()resultthat is not
identical()to itself:The published frame — all 6,310,390 rows and 12 columns — is
identical().The difference is entirely in the
.cb_extractsattribute, a list of fourtables (
fbs_new,fbs_old,cbs_crops,cbs_animals) that.read_production()attaches andbuild_primary_production()carries through.Each of the four holds the same rows with the same values in a different
order:
identical()identical()after sorting all columnsfbs_newfbs_oldcbs_cropscbs_animalsSo it is row order alone, not content. Dropping the attribute or sorting each
table makes the two runs byte-identical.
Why it matters
It is not a wrong number, but it costs reproducibility evidence. Any PR that
wants to assert "the output is unchanged" has to know to strip the attribute
first, and the natural check —
identical(build_primary_production(), baseline)— returns
FALSEon a change that moved nothing. That is exactly the shape ofcheck the epic has come to rely on (see #566 and the merge notes on #458), and
a control that fails against itself is worse than no control: the next agent
either weakens the assertion to
all.equal()or spends an hour looking for aregression that is not there. It cost about that here.
Where to look
.cb_extractsis set in.read_production()(R/build_production.R) from theCBS extraction step, before anything else in the pipeline runs, so it is
upstream of any polity or QC code. The likely candidates are an unordered
merge()/dcast()/rbindlist()or aby=grouping whose output order is notpinned — data.table does not guarantee group order without
keyby=or anexplicit
setorder().Suggested fix
Pin the order where the four tables are built (
setorderv()on their keycolumns, or
keyby=instead ofby=). It is a diagnostic side-channel, so theorder is not load-bearing for any consumer — the point is only that the object
should be a function of its inputs.
Verification is cheap and does not need this to be reproduced first: build twice
in two fresh sessions and assert
identical()on the whole object, attributeincluded.
Part of the polity migration epic #458 only incidentally; the defect is in
production I/O.