Conversation
cbuahin
commented
Aug 17, 2026
cbuahin
commented
Member
- Addressing legacy parity bugs
- Change of license
- Water quality (Eulerian and Langrangian), Heat transport, Multi-species Reactions
|
Water quality (Eulerian and Langrangian), Heat transport, Multi-species Reactions Thanks, this is very welcome. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…retire it The P1.4 check measured what the warning actually does: every extraction deck clamps while the system fills, because a near-empty element cannot satisfy any extraction. A deck extracting 40% of its incoming mass logged 108 clamps and 108 was not the interesting number -- zero of them indicated a problem. A notice that fires on ordinary decks is one users learn to ignore, which destroys it on the decks where it matters (lesson 148). The per-clamp warning is removed from all three seams -- node, age and ARD cell -- in the shared helper, so LEGACY, ARD and LARD stay identical rather than drifting apart to spare two of them an edit. The clamp stays observable through negsrc.clamp_events, negsrc.shortfall_mass and the end-of-run summary, which now also names the first element (the warning's only unique payload) and states that clamping during fill is expected, so a nonzero count is not misread as a modelling error. runtime_warned becomes first_clamp_recorded: it no longer gates a warning, only the first_node capture, and a flag called "warned" in code that does not warn is the trap this program keeps paying for. For the same reason OverExtractionClampsWarnsAndStaysNonNegative is renamed OverExtractionClampsSummarizesAndStaysNonNegative -- its body now asserts the warning's absence, so the old name described the opposite of what it checks. Two gate rows are deliberately flipped from "the warning fires" to "the warning is gone", keeping the observability claim rather than dropping it; the ARD row additionally pins that the clamp still reaches a user-visible channel, which its old needle only did incidentally. Verified by the checking agent at 4b26aa5: both flipped rows fail at base on all three engines and pass patched (4/4, 13/13); restoring the warning at the node seam alone fails only the three-engine row and at the cell seam alone only the ARD row; deleting the summary fails both, so the last observer is intact. Corpus 20/20 .out byte-identical, 0/20 .rpt moved. ctest 176/177 x3, the one failure pre-existing at HEAD. Protocol: plans/transport/P1_4B_CLAMP_WARNING_CONTRACT_HANDOFF_2026-08-29.md
…ing (#156) Adds the [OPTIONS] keys UNSTEADY_FRICTION / UF_K3 and FV_PRESSURE_CLOSURE across the whole option surface -- parser, InpWriter, GeoPackage reader and writer, the C API and the report preamble -- so a value a user sets survives every round trip instead of silently reverting to its default. The FV solver consumes unsteady friction as of this change: Router::initFv copies the dimensionless keys into FvOptions and kernels::ufUpdate applies the Vitkovsky/Brunone source term after the steady-friction stage. Under DYNWAVE the key is parsed, round-tripped and echoed but still inert. Virtual junctions gain initial-state seeding in the post-parse resolver: [VIRTUAL_JUNCTIONS] carries no init-depth column, so a VJ started dry even where its real neighbours defined an initial pool, leaving a hole at every splice. Each VJ head is now interpolated between the nearest non-virtual nodes along its spliced conduit chain. That seeding is DYNWAVE-only, and the restriction is measured rather than cautionary. Under FV the initial condition does not live in the node state -- Router::initFv lays each conduit's cells at a uniform depth -- so seeding the node alone starts the run with a VJ head its own cells contradict. On the mixed-flow study's e3 deck that drove flow-routing continuity from 0.000% to -19.133% with a 4371 m head excursion at VJ99. Making FV benefit needs the level-surface cell projection initFv deliberately does not do, which is its own change with its own gates. Gated by new test_fv_unsteady_friction, test_dw_unsteady_friction and test_fv_tpa_closure suites, plus option-surface and round-trip coverage.
Adds Infiltration2DView with its Infil2DDefaults / Infil2DRow / Infil2DCell records, and the SurfaceInfilMethod / SurfaceInfilDest enumerations mirroring the SWMM_INFIL2D_* codes in openswmm_infil2d.h. Only LOST is routed in this release (plan decision D-I4); the other two destinations parse so the grammar is stable and are rejected at validation.
The gate needs the POSIX fd and environment calls, which MSVC spells with a leading underscore and, for setenv/unsetenv, does not provide at all. Route them through small inline shims so the file compiles on Windows.
/plans/ stays out of the tree, but the snow divergence register is not a workplan -- it records every place this engine departs from legacy snow.c and why, which is what a user needs in order to interpret a deck comparison against EPA SWMM. Tracked by user decision (2026-08-21). The exclusion is spelled /plans/* rather than /plans/, because git cannot re-include anything underneath an excluded directory.
| print("MISSING: %s" % name) | ||
| bad += 1 | ||
| continue | ||
| have = open(path).read() |
The 2D cell rainfall was rebuilt only when 30 s or more had elapsed since the last refresh, so it lagged gage records. With 1-min records and a 7 s routing step, whole minutes were shifted or dropped: a 1-min on/off storm lost 12.3 % of its rain, on wet and dry cells alike. On Bellinge the 2D at the gage sites received 193.82 / 227.84 mm against the 1D subcatchments' 194.00 / 236.01 mm on the same gages. coAdvanceStep now also refreshes when any gage value differs from the values the field was built from (an O(n_gages) compare per step). The residual is one routing step per record change, as for the 1D. Bellinge re-run: 194.96 / 235.97 mm. Tests: DryCellsReceiveAndAccumulateRain (cells that never activate hold exactly the held-gage rain; fails without the fix) and RainPulseBetweenReportsAccumulates (a burst between 2D report instants reads 0 in Mesh2_face_rainfall but is carried by Mesh2_face_rain_cum).
…e one it enters
v6 rejected five networks legacy routes without complaint, with
ERROR 134 "illegal DUMMY link connections" — on nodes whose connections are
perfectly legal.
Under DW (and FV), an adverse-slope conduit is turned round so it runs
downhill: legacy conduit_reverse (link.c:1161-1193) and v6's own preprocessing
both physically SWAP node1 and node2 and flip `direction`. Every later reader
has to account for that swap, and legacy's counting loops do — they undo it:
degree count n = node1; if (direction < 0) n = node2; (toposort.c:82)
incoming mark j = node2; if (direction < 0) j = node1; (toposort.c:504)
so both counts are taken on the AUTHORED orientation. This check read the
post-swap node1 and node2 directly, which counts every uphill pipe as LEAVING
the node it actually enters.
MC_STP in noinflow-1200nodes is the clearest case. It is fed by three conduits
that climb from inverts of 17.15, 17.30 and 18.00 ft to its own 32.50 ft, so
all three were reversed. Counting their post-swap node1 gave MC_STP an outflow
count of 4 where legacy counts 1, and its single real outlet — a DUMMY conduit
to the outfall — was then rejected as an illegal connection. The whole 1,275
node model refused to run.
The same rule, with the same outfall redirect, was already being computed
correctly for ctx_.nodes.degree further down engine_open; this check simply
rolled its own count. It is not reused here only because it runs later.
Decks that legacy DOES reject still are, at legacy's node: 4275-nodes
(F12435), defaultlidvaluesinswc (Cisterns), riverhills (RH9-D), user1-dummy
(05yD77), user2-dummy (TW08010), user3-dummy (SMCP85-I). 3examples now agrees
with legacy exactly, reporting ERROR 134 at Node 1 where v6 previously raised
ERROR 145 instead.
Scored against the pinned Sep-5 legacy 5.3.0 oracle, baseline codex_verify39
(the hardened comparator, not the older lenient one):
noinflow-1200nodes ERROR-MISMATCH -> PASS, bit-exact
675-h-h-elements ERROR-MISMATCH -> PASS, bit-exact
seepage ERROR-MISMATCH -> PASS, bit-exact
14400-h-h-elements ERROR-MISMATCH -> PASS, bit-exact
3000-nodemodel ERROR-MISMATCH -> FAIL (runs now; diverges
numerically, a separate
defect this exposes)
banora ERROR-MISMATCH -> BOTH-ERROR (rejection parity)
3examples ERROR 145 -> ERROR 134 at Node 1, as legacy
Verified over the COMPLETE reachable set rather than by sweep alone: the check
can only fire on a deck carrying a DUMMY cross-section or an ideal pump, and the
corpus has 115 such decks, 49 of them in the scored short list. All 49 were
re-run: 31 PASS, 15 BOTH-ERROR, 3 FAIL, and ZERO regressions from PASS.
KNOWN REMAINING DIFFERENCE, not addressed here: on a deck with several illegal
nodes the two engines can name different ones. legacy runs checkDummyLinks to
completion inside toposort_sortLinks and stops at its first hit, BEFORE
validateGeneralLayout ever runs, whereas this check tests all three rules per
link in a single pass. user5-dummy is the case — v6 names 1SB11_OF, legacy
names 99. Both reject, so the verdict is unaffected (BOTH-ERROR), and matching
the message exactly needs the rules split into legacy's two separate passes.
ctest: the four standing reds (ard_node_store, msx_parity,
dw_unsteady_friction, xsect_parity), plus test_engine_inp_writer_atomic, which
fails at clean HEAD without this change — A/B confirmed, not from this commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 1D splits runoff steps at every rain-record boundary, so subcatchments receive the exact gage step function. The 2D applied the gage value current at the end of each routing window over the whole window, so a record change inside a window started the new rate up to one window early. It also reported Mesh2_face_rainfall as the rate over the window that ended at the report instant, while the .out reports the rate that starts there, so 2D plots lagged the gage and the .out by one record. - stepRunoff books each runoff step's gage rates (after the monthly adjustment) with SurfaceRouter2D::bookGageRates. Each 2D window takes each gage's depth integrated over the window divided by its length; a window with no rate change takes the rate itself, so the cell field is rebuilt only when a rate changes. - fillSurfaceSnapshot reports SurfaceRouter2D::reportRainfall: the .out's report-instant gage rates (gage::getReportRainfall) mapped through RAINFALL_MODE, with the per-cell rainfall forcings applied. Bellinge, 04:00-06:00 at 1-min reports: the gage-site cell's reported rain equals the .out subcatchment rainfall at all 121 records (1.2e-4 mm/hr), and it received 40.01459 mm against 40.01460 mm from the records. The dry-cell test now asserts the exact analytic depth, and the pulse test asserts the report-instant rate.
legacy junc_readParams (node.c) reads a [JUNCTIONS] row and then, under the
comment "check for non-negative values (except for invert elev.)", rejects a
negative max depth, initial depth, surcharge depth or ponded area with
ERROR 211 naming the offending token. The INVERT ELEVATION is exempt — it is
routinely negative. v6 read all five with to_double and validated none.
Accepting them is not a harmless leniency. A negative max depth drives a
pathological run:
1218-nodes six junctions carry maxDepth = -612 (e.g. LOADING_MH_3B
612.000000 -612.000000). legacy refuses the deck in 24 ms;
v6 accepted it and simulated 21 days straight past the
harness's 1,800 second cap. It had been sitting in the
rejection audit as a v6 TIMEOUT — read as a performance
problem, when the engine was in fact running a model legacy
never lets start.
960-nodes 180 rows with maxDepth = -0.100000; v6 ran it to completion
and was scored ERROR-MISMATCH.
Both now reject at the same token legacy names:
1218-nodes RUN-ERROR -> BOTH-ERROR (ERROR 211: -612.000000)
960-nodes ERROR-MISMATCH -> BOTH-ERROR (ERROR 211: -0.100000)
Verified over the COMPLETE reachable set rather than by sweep: the check can
only fire on a [JUNCTIONS] row whose 3rd-6th column is negative, and the corpus
has 13 such decks, 11 in the scored short list. All 11 were re-run. The other
nine were ALREADY BOTH-ERROR, so nothing could regress — no deck carrying a
negative junction parameter has ever passed, because legacy rejects every one
of them. Zero regressions from PASS.
KNOWN TEXT DIFFERENCE, deliberately not addressed: legacy's message is
"ERROR 211: invalid number %s " — a trailing SPACE and no full stop — because
it appends its locus, "at line %ld of %s] section:" (FMT18). v6 writes
"invalid number %s." with a full stop and no locus, since errors here do not
carry the input line number at all. Both engines reject, so the verdict is
BOTH-ERROR either way and no .out comparison is affected; threading the line
number through would be its own change.
ctest: the four standing reds (ard_node_store, msx_parity,
dw_unsteady_friction, xsect_parity) plus test_engine_inp_writer_atomic, which
fails at clean HEAD without this change and is not from this commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…r, and D-A20's age gate Four defects on the 1D <-> aquifer seam, each found by a gate written for it and each falsified before landing. G-W1 -- the node bed delivered 9.4x the water the aquifer released. The cap on an aquifer->node drain measured the cell's drainable water with the textbook specific yield (theta_s - theta_r) while the column that spends it uses the closure-consistent theta_s - theta_bot floored at kSyFloor. For a table a few centimetres under the ground those are 0.35 and 0.001, so the cap sat 350x above anything reachable and never bound: the column went short on essentially every firing and the shortfall was refunded into the AQUIFER's books alone, after the router had already handed the volume to the node. Both continuity blocks closed and the seam still created water -- 0.625 m3 delivered against 0.067 m3 released on the gate deck, 89 % of it refunded. Specific yield now has one owner, SubsurfaceSolver::specificYield, which the drain cap and the column both call; refunds go 120 -> 0 and the two sides agree to the last digit. The node->aquifer headroom cap deliberately keeps the old yield: making it consistent was attempted twice and reverted twice, and the reasons are at the call site. T7.4 -- the aquifer quality block divided its residual by a three-term copy of "what came in" written before T7.4 added two inflow routes, printing 6198889.840 % beside a balance exact to the last digit. The sum is now SubsurfaceTransportState::continuityDenominator, beside the residual it scales. And when there is no scale at all the block prints n/a rather than 0.000, because a zero scale beside a non-zero residual is how the next unbooked route will announce itself. T7.4b -- the aquifer->node transfer was booked in two inflow rows at once (the queue AND qual_routing_gw_in). 49.7 % continuity error on a deck whose whole quality budget is aquifer -> node -> outfall. D-A20 -- ET's effect on water age in the aquifer was implemented and gated by nothing: the only ET gate asserted that the SOLUTE row lost nothing, so reverting the kernel to the GW transport plan's superseded "ET removes water not age-volume" would have left every gate green. Gates: test_engine_gw_transport_kernel 18 -> 19, including a second closure (ENSLAVED) because every fixture was CLOSED_FORM, and an instrument guard -- the drain-cap clamp counter -- so a deck that cannot see the defect fails loudly instead of passing quietly. A bed deck is not automatically a regression deck for this: one scoring 56.8x on the specific-yield ratio was measured bit-identical against a reverted build. Validated on macOS across four rounds: kernel and 2d_aquifer green, ctest 217/221 with the failure set unmoved. NOT included and still open: G-W2/G-W4 (kSyFloor used as a capacity on both sides of the seam -- see the handoff's table), and R-MI, a 1D router continuity defect under a sharply decaying micro-inflow that reproduces with no aquifer, no mesh and no subsurface solver in the process. CHANGELOG entries are deliberately held back: a peer has a docs pass staged that relocates entries in the same file.
… as committed f96e5a5 shipped EtCarriesTheAgeRowOutWithTheWaterD_A20 without ever compiling it, and the risk named in that commit's own message materialised: the gate fails as committed. `[GW_TRANSPORT_OPTIONS] TRANSPORT_AGE YES` is only a MASK — TransportPolicy computes `e.age = ta && ctx.options.water_age` — so the deck also needs the project-level `[OPTIONS] WATER_AGE ON`, or the aquifer's age_row stays -1 and there is nothing to measure. The gate caught this itself rather than passing vacuously, which is what ASSERT_GE(age_row, 0) is for. Now verified rather than reasoned: compiles clean, passes, and falsified — reverting fireCellSpecies to the superseded "ET removes water not age-volume" spec (GW plan §2.4 / §3.5, amended 2026-09-27) fails it with the diagnosis in the failure message. test_engine_gw_transport_kernel 18/19 in a Linux sandbox; the one red is HotStartCarriesTheAquiferSpeciesAcrossARestart, which throws "filesystem error: cannot remove ... Operation not permitted" on this sandbox's mount -- the same restriction that stops git unlinking in this tree. Not a code defect; green on macOS. 2d_aquifer 22, transport_policy 7, gw_transport_authoring 7, 2d_output_options 15 -- all unchanged.
…in legacy
handle_xsections looked the shape keyword up in an unordered_map and, when the
lookup missed, simply did nothing:
auto it = SHAPE_MAP.find(shape_str);
if (it != SHAPE_MAP.end()) ctx.links.xsect_shape[idx] = it->second;
The link kept whatever shape it already had -- the default -- and the geometry
columns were then parsed into that default. So a deck naming a cross-section
SWMM does not have ran to completion against the wrong pipe and reported
success. legacy has never allowed it: link.c:190 is
k = findmatch(tok[1], XsectTypeWords);
if ( k < 0 ) return error_setInpError(ERR_KEYWORD, tok[1]);
Two real spellings reach this path in the corpus, and both are silent today:
SEMI_ELLIPTICAL / SEMI_CIRCULAR legacy's keywords are w_SEMIELLIPTICAL
and w_SEMICIRCULAR (text.h:247,249) --
no underscore. legacy supports both
shapes fully (xsect.c:339,359); the decks
just spell them with an extra character.
UNKNOWN what an InfoWorks CS export writes when
it cannot map a shape ("; Cannot find
equivalent for InfoWorks CS shape
CATENARY" sits above every such row in
large-rivers).
The keyword resolution is now legacy's, not an exact-match map. findmatch
returns the FIRST table entry that is a PREFIX of the token (input.c:791-823:
match(str, substr) walks substr against the head of str), so the table order is
part of the semantics and SHAPE_WORDS is now in XsectTypeWords order
(keywords.c:160-184). This also makes "DUMMY_2.1" read as DUMMY, as legacy
reads it.
user1/3/5-semi-elliptical ERROR-MISMATCH -> BOTH-ERROR
user1/3/5-semi-circular ERROR-MISMATCH -> BOTH-ERROR
user1/3/5-every-shape ERROR-MISMATCH -> BOTH-ERROR
user1/3/5-all-closed-shapes ERROR-MISMATCH -> BOTH-ERROR
v6 now names the same code and the same token legacy does, e.g.
"ERROR 205: invalid keyword SEMI_ELLIPTICAL".
Verified over the COMPLETE reachable set rather than by sweep. The behaviour can
only change on an [XSECTIONS] row whose shape token is not an exact SHAPE_MAP
key, so every such token in the corpus was enumerated (both engines tokenize on
whitespace, so the enumeration is what each actually sees). Outside the twelve
decks above they occur only in:
routing-mixed-shapes, don, user2/4-* already BOTH-ERROR
5000-subcatchments, large-rivers not in the scored list
corpus/hydraulics/sewer-model not the scored copy of that id -- its
link names contain spaces, so legacy
collapses them and rejects the deck
with ERROR 207 (confirmed by running
it); the scored sewer-model is
corpus/epa/sewer-model, whose
[XSECTIONS] is clean
No scored PASSing deck carries a shape token legacy rejects, so no PASS can
regress. Confirmed empirically as well: 77 PASSing decks covering every entry
whose table position moved -- including extran1-semi-elliptical and
extran1-semi-circular, which use the correct spellings, and the
RECT_TRIANGULAR, DUMMY, CUSTOM, STREET, IRREGULAR, BASKETHANDLE,
MODBASKETHANDLE, POWER, RECT_ROUND, CATENARY and GOTHIC users -- were re-run
and all 77 are still bit-exact.
The "RECT_TRIANG" alias is dropped: legacy has only w_RECT_TRIANG =
"RECT_TRIANGULAR", and under the prefix test a bare "RECT_TRIANG" matches
nothing, so legacy answers ERROR 205. No corpus deck uses it, and v6's own
writer emits RECT_TRIANGULAR.
STILL OPEN, the other two halves of this bucket -- both separate sites, so not
folded in here:
user1/3/5-modbaskethandle legacy ERROR 211 with an EMPTY token, from
link.c:250 when xsect_setParams returns FALSE.
For MOD_BASKET that is "if (p[1] <= 0.0)"
(xsect.c:451) and these decks give Geom2 = 0.
v6 has no equivalent geometry-validity gate.
user3/5-custom-shape legacy ERROR 209 from link.c:229 --
project_findObject(CURVE, tok[3]) fails because
the deck never defines a shape curve named
CustomShape (its [CURVES] holds only STORAGE
curves). v6 defers the curve lookup and does
not error when it cannot be resolved.
ctest in build/gate: 216/221, the four standing reds (ard_node_store,
msx_parity, dw_unsteady_friction, xsect_parity) plus test_engine_inp_writer_
atomic, which fails at clean HEAD and is not from this commit. xsect_parity is
a geometry-function parity test that never references the [XSECTIONS] reader
and fails in 0.01 s; it was already red at this build before this change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… error
legacy resolves a CUSTOM link's shape curve while it reads the row and refuses
the deck when the curve is absent (link.c:225-231):
i = project_findObject(CURVE, tok[3]);
if ( i < 0 ) return error_setInpError(ERR_NAME, tok[3]);
v6 cannot resolve it there, because [CURVES] may be parsed after [XSECTIONS],
so the name is stashed and looked up in resolve_cross_references. But the
deferred lookup had no failure branch: when find_curve missed, the loop simply
fell past the whole tabulation block. The link kept XsectShape::CUSTOM with no
shape table behind it and ran at zero area -- the silent-zero shape again, and
with no diagnostic anywhere in the report.
The IRREGULAR transect lookup immediately above this one already had exactly
this check, for exactly this reason; CUSTOM just never got it. This adds the
matching else branch.
user2-custom-shape already BOTH-ERROR, now for legacy's reason
user3-custom-shape ERROR-MISMATCH -> BOTH-ERROR
user5-custom-shape ERROR-MISMATCH -> BOTH-ERROR
All three now print what legacy prints, "ERROR 209: undefined object
CustomShape". The decks reference CustomShape from 12 [XSECTIONS] rows each and
never define it -- their [CURVES] section holds only STORAGE curves -- so legacy
is right to refuse them and v6 was wrong to run them.
find_curve matches legacy's object lookup in the way that matters here: legacy
project_findObject(CURVE, ...) finds a curve of ANY subtype, and so does
find_curve (TableData::find_by_kind with want_timeseries = false). A STORAGE
curve named CustomShape would satisfy both engines; neither deck has one.
Verified over the COMPLETE reachable set. The new branch can only fire on a link
whose shape is CUSTOM and whose curve name resolves to nothing, so every CUSTOM
row in the corpus was enumerated against its deck's [CURVES] section. Exactly
six decks carry a dangling name, all of them CustomShape: the three above plus
user2/3/5-every-shape, and the every-shape trio is already BOTH-ERROR. NO deck
has a CUSTOM row short enough to leave the name empty, so the empty-name path
stays as it was. No PASSing deck has a dangling CUSTOM curve, so no PASS can
regress.
Confirmed empirically too: 85 PASSing decks were re-run and all 85 are still
bit-exact. That set is every one of the 16 PASSing decks that use a CUSTOM
cross-section -- including user1-custom-shape and user4-custom-shape, which are
the same decks as the three above but with the shape curve actually defined --
plus the 77 decks covering the keyword table reordered in f370269.
KNOWN REMAINING DIFFERENCE, deliberately not addressed: legacy also rejects a
CUSTOM row whose Geom1 is absent or <= 0 (link.c:226, ERROR 211 naming tok[2]).
v6 reads it with to_double and only skips the tabulation when y_full <= 0. No
corpus deck exercises it, so it is recorded rather than fixed unverified.
STILL OPEN in this bucket, the last three decks: user1/3/5-modbaskethandle.
legacy raises ERROR 211 with an EMPTY token from link.c:250, which is what
xsect_setParams returning FALSE reports; for MOD_BASKET that test is
"if (p[1] <= 0.0)" (xsect.c:451) and these decks give Geom2 = 0. v6 has no
geometry-validity gate for any shape, so closing it is a wider change than this
one.
ctest in build/gate: 216/221, the four standing reds (ard_node_store,
msx_parity, dw_unsteady_friction, xsect_parity) plus test_engine_inp_writer_
atomic, which fails at clean HEAD and is not from this commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A 20 x 10 m mesh with three gages hundreds of metres away, surrounding it (natural-neighbour weights) and all to one side (inverse-distance fallback), in both RAINFALL_MODEs. Every cell must get a normalized weight row over the outside gages (all three under natural neighbour, the closest under nearest), and the rain delivered after a step must equal the weighted gage rates.
xsect::setParams is a faithful port of legacy xsect_setParams, down to
answering -1 where legacy answers FALSE -- "I cannot build a section from these
numbers". legacy treats that as fatal: link.c:250 turns it into ERROR 211,
naming an empty token, and the deck does not run.
v6 computed the same verdict and threw it away. The one caller,
resolve_cross_references, tested `rc == 0 && xs.a_full > 0.0` and sent every
other case to a rectangular fallback:
a_full = w_max * y_full;
So a MODBASKETHANDLE with a top width of 0 -- a modified basket handle with no
basket, which has no geometry at all and which setParams had already refused at
XSection.cpp:457 -- silently ran as a rectangle.
rc is what must be tested, not the branch. DUMMY reaches the same else-branch
with rc == 0, because setParams deliberately leaves a dummy's fields to the
caller (its own default case returns 0 for DUMMY, IRREGULAR, CUSTOM and STREET),
and a dummy conduit must keep running. Gating on the branch would have rejected
every deck with a DUMMY link -- 55 of them, 19 currently bit-exact.
user1-modbaskethandle ERROR-MISMATCH -> BOTH-ERROR
user3-modbaskethandle ERROR-MISMATCH -> BOTH-ERROR
user5-modbaskethandle ERROR-MISMATCH -> BOTH-ERROR
user2/4-modbaskethandle already BOTH-ERROR, now for legacy's reason
With this and the two commits before it, the ERROR-MISMATCH bucket is EMPTY:
v6 and the pinned Sep-5 legacy 5.3.0 oracle now agree on every deck either one
refuses.
Verified over the COMPLETE reachable set. Surfacing rc can only reject a deck
where a guard v6 ALREADY HAS fires, so every [XSECTIONS] row in the corpus was
evaluated against those guards -- FILLED_CIRCULAR Geom2 >= Geom1, RECT_TRIANG,
RECT_ROUND and MOD_BASKET Geom2 <= 0 and their derived "arc deeper than the
pipe" test, and the ellipse/arch standard-pipe size codes (v6's code selection
is character-for-character legacy's, including the substitution of Geom1 for the
code when Geom2 is 0, so the enumeration is exact). Thirty-nine decks are
reachable. THIRTY-NINE ARE ALREADY BOTH-ERROR OR ERROR-MISMATCH; none is
PASSing, so no PASS can regress. A full 1,071-deck sweep confirms it.
The derived arc test is unit-invariant -- rBot, wMax and yFull all carry the
same length factor and the test is a comparison between two of them -- so it
cannot fire on one unit system and not the other.
KNOWN REMAINING DIFFERENCES, recorded rather than fixed unverified. v6's
setParams is missing seven of legacy's direct parameter tests:
every non-DUMMY shape p[0] <= 0 (xsect.c:439)
RECT_CLOSED p[1] <= 0
RECT_OPEN p[1] <= 0; p[2] outside [0,2]
TRAPEZOIDAL any of p[1..3] < 0; zero bottom width AND zero
side slopes
TRIANGULAR, PARABOLIC p[1] <= 0
POWERFUNC p[1] <= 0 or p[2] <= 0
Thirty-one corpus decks trip one of these in legacy -- overwhelmingly Geom1 = 0
on a CIRCULAR or RECT_CLOSED row, which InfoWorks and ICM exports emit freely --
and every one of them is ALREADY BOTH-ERROR for another reason, so adding the
tests moves no verdict on this corpus and buys no parity today.
A TRAP for whoever adds them, found while proving the safety of this commit:
legacy zeroes Geom3 and Geom4 before calling setParams when the link is NOT a
conduit and the shape is RECT_OPEN (link.c:241-245). v6 has no such rule.
211-h-h-elements is bit-exact today and has two WEIRS carrying
"RECT_OPEN 20 10 3 3" -- a sides-to-ignore count of 3, which legacy's own
RECT_OPEN test rejects. legacy accepts the deck ONLY because the zeroing fires
first. Adding the RECT_OPEN test without the zeroing rule regresses that deck.
Also noted: legacy stops reading input after MAXERRS = 100 errors and writes
FMT19 (input.c:36,136,245-251). v6 has no cap and accumulates every one, so on
a deck like 10103-subs-nodes -- 9,708 rows with Geom1 = 0 -- the two engines'
error LISTS differ in length even where both refuse the deck. Pre-existing, and
it does not change a verdict.
ctest in build/gate: 216/221, the four standing reds (ard_node_store,
msx_parity, dw_unsteady_friction, xsect_parity) plus test_engine_inp_writer_
atomic, which fails at clean HEAD and is not from this commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ver asked for one
A deck can carry pollutants with no [LANDUSES] at all. Rainfall itself is a
source: [POLLUTANTS] column 3 is Cppt, the concentration in precipitation, and
legacy deposits it on every subcatchment surface through findPondedLoads
(surfqual.c:428, wRain = Pollut[p].pptConcen * LperFT3 * vRain). Nothing about
that needs a land use — only findWashoffLoads does.
legacy's guard is exactly two conditions (surfqual.c:283-286):
if (Nobjects[POLLUT] == 0 || area <= 0.0) return;
v6 required a land use in BOTH halves of the path:
stepSurfaceQuality if (n_pollutants() > 0 && n_landuses() > 0)
bookWashoffLoads if (np <= 0 || ns <= 0 || n_landuses() <= 0 || ...) return;
so on a deck with pollutants and no land uses the entire surface-quality step
was skipped. No wet deposition, no ponded mixing, no LID loads, nothing booked.
And it was SILENT: with every term zero the runoff-quality continuity error is
0.000 %, so the report looked perfect while the engine transported no pollutant
from any source.
lid1d is the case. Four pollutants, Cppt of 30 / 66 / 0.04 / 5, no [LANDUSES],
no [BUILDUP], no [WASHOFF] — rainfall is the only source. legacy runs it; v6
reported 0.000 for every pollutant on every subcatchment, node and link from
the first wet period, and closed the books at 0.000 %.
Runoff Quality Continuity, BOD, kg legacy v6 before v6 now
Wet Deposition 119606.545 0.000 119606.545
Infiltration Loss 6257.229 0.000 6250.704
BMP Removal 2533.592 0.000 2533.592
Surface Runoff 110015.701 0.000 110160.094
Continuity Error (%) -0.485 0.000 0.554
Wet deposition and BMP removal now match legacy exactly. The .out divergence
falls 5,829,369 -> 5,186,082 cells and its first diverging period moves 181 ->
189, where it now coincides with the link.FLOW divergence this deck also has —
a separate hydraulic defect, not addressed here.
The land-use loop was already correct at nlu == 0: it simply does not execute,
which is what legacy's findWashoffLoads does without coverages. `nlu` is used
nowhere else in the function but as that loop's bound and its coverage index.
Verified over the COMPLETE reachable set. The gate can only change behaviour on
a deck that has pollutants and NO land uses — with land uses present the guard
was already open — so every such deck in the corpus was enumerated and re-run:
26 scored cases, 22 PASS and 4 FAIL. All 22 PASS decks are still bit-exact, and
of the four FAILs lid1d improves while 2015-h-h-elements, emc-exam1 and
wq-extran1 are unchanged. ZERO regressions.
KNOWN REMAINING DIFFERENCE on this deck, pre-existing and already documented in
the code: legacy books the residual ponded mass at end of run as Remaining
Buildup (1379.646 kg); v6 leaves that term 0.000 on both paths. That accounts
for most of what is left of the 0.554 % error.
Also noticed, NOT addressed here: v6 labels the Runoff Quality Continuity
column `lbs` on this SI deck where legacy writes `kg`, while the VALUES are
metric and match legacy's. The unit label is wrong, not the arithmetic. v6 also
prints one table per pollutant where legacy prints all pollutants as columns of
one table. Report formatting only; the .out is unaffected.
ctest in build/gate: 216/221, the four standing reds (ard_node_store,
msx_parity, dw_unsteady_friction, xsect_parity) plus test_engine_inp_writer_
atomic, which fails at clean HEAD and is not from this commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- RainfallInterpolator.cpp: include <string> for std::to_string. MSVC 14.51 (VS 18) no longer pulls it in via <stdexcept>, so the engine target failed to compile on windows-2025 (regressed in e7fc5fe). - test_binding_completion: compare the per-element air temperature with pytest.approx. SI input is stored as degF, and the C->F->C round trip returns 21.000000000000007 unless the compiler fuses it into an FMA. - test_concurrent_simulation: bring test_bulk_getters_concurrent_reads in line with the NativeAccess contract from 8a7545b. A second thread is refused with LifecycleError instead of blocking, so readers now retry on that refusal, still compare every read bit-for-bit with the serial baseline, and fail if any reader hangs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013vyx6gNkSKqGEuZj1L9UBR
… keeps it
legacy handles a negative regulator offset with three different hands, none of
them the conduit's, and the order is part of the semantics:
parse link_setParams (link.c:366-391) mirrors the authored offset1
into offset2 for orifices, weirs and outlets — raw, sign and all
validate orifice_validate (link.c:1719) and weir_validate (link.c:2188)
zero a negative offset1 SILENTLY; outlets are left alone;
conduit_validate (link.c:1061-1070) is the only place WARNING 03
is printed in DEPTH mode, and the only one that touches offset2
raise the WARNING-10 lift to the downstream invert (link.c:424-439)
moves offset1 only
offset2 is never revisited, so for a regulator it holds the authored value
forever — and the node crown-elevation pass (dynwave.c:143-152) reads it RAW.
An orifice authored with a large negative offset therefore contributes a crown
far BELOW its node and never raises that node's EXTRAN surcharge threshold.
v6 zeroed both ends of every link in DEPTH mode, each with WARNING 03, and the
WARNING-10 block then mirrored the POST-zeroing crest into offset2. On
subcatchments-no-infiltration that lifted junction ST2337's crown from
legacy's 1.75 ft (conduit 334.1's crown) to 2.0 ft (the zeroed orifice
INLET's). The two engines marched bit-for-bit for 776 one-minute report
periods — identical adaptive step grids, identical link flows, identical qsum
— until the junction's depth crossed 1.75 ft at t = 46,620 s, where legacy
switched it to the surcharged dqdh depth update one step before v6 did. The
SWMM_TRACE_RSTEP trace shows the signature exactly: at step 6496 qhash and
qsum still agree and only yhash differs. From there the deck carried a
permanent ~0.1 % flow bias: FAIL with 298,784 cells over tolerance.
The fix restores legacy's order at the WARNING-10 site — mirror the authored
crest into offset2 FIRST, apply the validators' silent zeroing SECOND, raise
LAST — and reduces the DEPTH-mode zeroing loop to what conduit_validate
actually does: conduits only, WARNING 03 only there. In ELEV mode the crest
reaching the mirror is already converted and clamped non-negative by
legacyOffsetHeight, so the mirror equals link_convertOffsets' and the new
zeroing never fires; ELEV decks are bit-identical.
subcatchments-no-infiltration FAIL (298,784 cells) -> PASS, bit-exact
Verified over the COMPLETE reachable set rather than by sweep alone: behaviour
can only change on a DEPTH-mode deck carrying a negative offset on an orifice,
weir or outlet, and the corpus holds exactly five — the deck above,
1950-h-h-elements (unchanged: its divergence is the deck-wide oscillation, not
this crown), rules (BOTH-ERROR, unchanged), and 48-h-h-elements and
linear-force-main, both PASS before and after (48-h-h-elements carries the
IDENTICAL INLET -888.05 pattern; its junction never enters the 1.75-2.0 ft
window where the two crowns disagree). No scored deck has a negative WEIR or
OUTLET offset, so the outlet-zeroing removal and the weir re-ordering move no
verdict; they are covered by the same five-deck argument plus a 70-deck
regression batch over PASSing regulator decks in both offset modes, all still
bit-exact.
Also fixed by the same change, cosmetic: v6 no longer prints WARNING 03 for a
negative regulator offset legacy zeroes silently — 1950-h-h-elements' report
carried a "WARNING 03: negative offset ignored for Link cso2_grate" line that
legacy's does not.
ctest in build/gate: 216/221, the four standing reds (ard_node_store,
msx_parity, dw_unsteady_friction, xsect_parity) plus test_engine_inp_writer_
atomic, which fails at clean HEAD and is not from this commit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… and seed v6's street sweeping (Gap #34) never removed anything on runoff46-sw5 — legacy sweeps 886 lbs of COD there, v6 0.000, and every washoff concentration ran high by that margin: each pollutant a constant factor (COD x1.212, TSS x1.248, TOT.NIT x1.285, O-PO4-P x1.1428, TOT.SOL x1.250 — one minus availability x sweeping efficiency, per pollutant). Three independent deviations from legacy, each enough to break it: 1. THE CLOCK STARTED AT ZERO. legacy landuse_initState (landuse.c:377) seeds lastSwept = StartDateTime - sweepDays0, so a deck due for its first sweep at t = 0 (runoff46-sw5: interval 1 day, days-since-last 1) sweeps at the first eligible step. v6's days-since-last-sweep accumulator began at 0 and ignored the [LANDUSES] LastSweep column, so the first sweep could come no earlier than one full interval into the run — never, inside this deck's window. 2. THE CLOCK ONLY TICKED ON DRY, IN-SEASON STEPS. legacy's test is `aDate - lastSwept >= interval` (surfqual.c:228) — calendar dates. A sweep can come due during a storm or out of season and fires at the next eligible step. v6 advanced the accumulator only when its whole gate passed, so every rainy day pushed the entire schedule back. 3. AVAILABILITY WAS DIVIDED BY 100. The [LANDUSES] Availability column is a FRACTION, read raw by legacy (landuse.c:70, with :81 rejecting anything outside [0,1]); only the [WASHOFF] sweeping efficiency is a percent. v6 divided the fraction by 100 too, so the sweeps that did fire removed 200x too little. Also aligned while here, from the same legacy lines: - The rain gate is the SUBCATCHMENT's own rainfall vs MIN_RUNOFF (runoff.c:284), not "any gage raining": a subcatchment whose gage is dry sweeps while another's storm runs, and a drizzle below MIN_RUNOFF still permits sweeping. v6 used the global IsRaining flag. - Both sweep bookings (pollutants and BW-MSX species) now use legacy's exact form — clamp the new store into [0, old] first, book the clamped difference (surfqual.c:236-242) — so the ledger closes for any input, not only availability x efficiency inside [0,1]. runoff46-sw5 FAIL (673 cells) -> PASS, bit-exact Verified over the COMPLETE reachable set: sweeping can only act on a deck whose [LANDUSES] carries a sweep interval > 0, and the corpus holds six. runoff46-sw5 converts; allwq (availability 0 — sweeps remove nothing in either engine), runoff31-sw5 and runoff41-sw5 (interval 30 days, days-since-last 15 — first sweep falls beyond the simulated window) are still bit-exact; storm-cut and swmm-studyarea-modelling stay BOTH-ERROR. test_msx_buildup_washoff's LedgerCloses fixture authored availability as "60" — a value legacy rejects at parse — which the /100 happened to turn into the intended 0.6, and its comment explicitly leaned on the zero seeding ("a pre-existing parity gap ... hence the short interval rather than LastSweep"). The fixture now writes 0.6 and the comment describes the legacy-faithful schedule; all seven tests pass. NOT addressed here, recorded: legacy rejects [LANDUSES] rows with sweepInterval < 0, availability outside [0,1] or days-since-last < 0 with ERR_NUMBER (landuse.c:79-84); v6 accepts them. No corpus deck exercises it (the one out-of-range author was our own fixture, above). ctest in build/gate: 216/221, the four standing reds (ard_node_store, msx_parity, dw_unsteady_friction, xsect_parity) plus test_engine_inp_writer_ atomic, which fails at clean HEAD and is not from this commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rries it
When one subcatchment discharges onto another, legacy sends the pollutant
with the water: subcatch_getRunon adds the donor's
Subcatch[k].newQual[p] += oldRunoff * oldQual[p] * LperFT3 (subcatch.c:553)
into the receiver's accumulator, and the receiver's ponded complete-mix
consumes it as wRunon (surfqual.c:442-443), as does the full-LID path
(surfqual.c:560-561). v6 sent the WATER (runon_inflow, in v_inflow) and no
load at all — the ponded block said so in its own comment, "wRunon ~= 0
(upstream subcatch concentrations not propagated here)" — so the receiver
both missed the incoming mass AND diluted its own washoff with the donor's
clean water.
washoff-runon is the minimal case: subcatchment 1 (EMC 10 mg/L) runs onto
subcatchment 2 (EMC 10 mg/L), and legacy's reported concentration at 2
climbs from 10 through the ponded mix while v6 sat at a flat 10.000 from
the first period.
The accumulator is a new per-(subcatch, pollutant) field,
subcatches.runon_qual_rate (mg/sec), zeroed and rebuilt by assembleRunon()
beside runon_inflow from the same previous-substep values: runoff[ui] there
still holds legacy's oldRunoff, and conc_old is the donor's rolled washoff
concentration, legacy's oldQual after setOldState. Both legacy consumers now
read it:
- the ponded complete-mix adds wRunon = runon_qual_rate * dt;
- the full-LID runon term reads the SAME accumulator, replacing a stand-in
that multiplied the runon water by the RECEIVER's own old concentration
— right only when donor and receiver happened to share an EMC.
washoff-runon FAIL ( 358 cells) -> PASS, bit-exact
lid-master-sustain FAIL (3,614 cells) -> PASS, bit-exact
suds-swmm5-example4 FAIL (3,613 cells) -> PASS, bit-exact
The two LID decks are five-deep subcatchment cascades feeding LID units —
the receiver-concentration stand-in and the missing ponded term both bound
on them.
Verified over the COMPLETE reachable set: the new load is nonzero only where
a subcatchment with pollutants discharges onto another, and the LID term's
source only changes where a full-LID subcatchment receives run-on — every
pollutant deck with sub-to-sub routing or LID usage, 27 in the corpus. Three
convert above; all seven PASSing decks in the set (duobiocell, lid-modeling,
master-gw, more-process-100-bio-cell, road, variations-on-emc,
wq-w-wo-rg-2subcatchments) are still bit-exact; lid1d keeps its earlier
improvement; the rest are unchanged. ZERO regressions.
KNOWN REMAINING GAPS in the same family, recorded not fixed — the loads that
reach a subcatchment by paths other than surface run-on still arrive with no
pollutant attached:
- an outfall routed onto a subcatchment: legacy books Outfall.wRouted into
the same accumulator plus DEPOSITION_LOAD (runoff.c:525-532); v6 tracks
the routed WATER (outfall_runon_vol) and has no wRouted at all;
- LID underdrains sent to another subcatchment (legacy lid_addDrainRunon's
quality side).
No corpus FAIL hinges on either today.
ctest in build/gate: the four standing reds (ard_node_store, msx_parity,
dw_unsteady_friction, xsect_parity) plus test_engine_inp_writer_atomic,
which fails at clean HEAD and is not from this commit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… LID report file
LIDGroupSoA carried its unit-conversion pair as HARD-CODED US defaults —
double ucf_raindepth = 12.0;
double ucf_rainfall = 43200.0;
— and nothing ever assigned them. They are not display factors: legacy's
underdrain equation works in USER units (lidproc.c:1399-1414, "compute drain
outflow from underdrain flow equation in user units"), so getStorageDrainRate
does head *= ucf_raindepth and outflow /= ucf_rainfall, and the
roof-disconnection cap is drain_coeff / ucf_rainfall (lidproc.c:484). On an
SI deck every LID underdrain therefore ran the drain law with in/hr factors:
for the common exponent 0.5 that is sqrt(12/304.8) x (25.4x rate factor) =
5.04x too much drain flow.
usgs-runoff (FLOW_UNITS LPS) is the worked case. Its rain barrels drained
five times too fast, never filled, never overflowed: the LID Performance
Summary showed identical Total Inflow (363.44 mm) split 96.27 surface +
261.17 drain in legacy against 0.00 + 357.44 in v6, and the .out was
bit-exact until the exact minute the storage level first crossed the
6.0-inch drain offset, then permanently biased. The fix is one assignment
per group from the deck's own factors (already computed two lines up for
the layer-parameter conversions).
The instrument that found it is the second half of this commit, and a
missing parity feature in its own right: v6 wrote NO per-LID report file
([LID_USAGE] column 9). It now writes legacy's exactly — createLidRptFile +
initLidRptFile's header (US and SI unit rows), lidproc_saveResults' twelve
columns, formats (including the lone %8.4f evap column), and the dry-spell
compression (held row, wasDry seeded 1). On the rain-barrel repro the two
engines' 994-row files now differ by ONE character: legacy preserves the
[TITLE] line's trailing space and v6's title store trims it. The kernel
rates the row needs (SurfaceInfil, PavePerc, SoilPerc) are staged per unit
by runUnitLegacy; soil moisture prints 0 for a unit with no soil layer,
as legacy's x[SOIL] stays.
Minimal repros (kept in benchmarks results/short_sweep/manual/usgs_ablate/):
A_nolid (CN+aquifer, no LID) bit-exact before and after
C_RAINGARDEN / C_SWALE bit-exact before and after
C_RB / C_RB_d0 / C_TRENCH 492 / 1001 cells -> ALL bit-exact
B_lid (all four types) 1000 cells -> bit-exact
Scored corpus, all 67 decks with LID usage re-run:
swmm-5-model-with-lid-but-no-bottom-infiltration FAIL 7,172 -> 261 cells
usgs-runoff FAIL 7,570 -> 1,470
lid1d FAIL 5,829,369 -> 5,060,044
lid-master-sustain, suds-swmm5-example4 still PASS (1d63ab4's converts)
all 36 previously-PASSing LID decks still bit-exact
ZERO regressions
No US deck changes (identical factors), and an SI deck that PASSed before
the fix can only have been passing because its drains never engaged, which
the 23 SI decks in the set confirm empirically. The residues that remain on
usgs-runoff and swmm-5-model-with-lid are the next seam, now reachable with
the report files on both sides.
ctest in build/gate: 216/221, the four standing reds (ard_node_store,
msx_parity, dw_unsteady_friction, xsect_parity) plus test_engine_inp_writer_
atomic, which fails at clean HEAD and is not from this commit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ateral
legacy seeds the quality-inflow denominator with the NET lateral flow,
floored at zero:
Node[j].qualInflow = MAX(0.0, Node[j].newLatFlow); (routing.c:480)
and dynwave's Node.inflow carries the same floored net into
qualrout_execute. newLatFlow is the SUM of every lateral component — runoff,
DWF, RDII, external, LID drains, and two-way groundwater exchange, which can
be NEGATIVE when the aquifer draws from the channel. A withdrawing component
therefore shrinks the carrier volume UNDER the remaining lateral mass, and
the node's mixed concentration legitimately rises ABOVE every source
concentration.
v6 summed each positive source's own carrier volume into qual_vol_in and
skipped negatives entirely, so the withdrawal never touched the denominator
and the node could never exceed its sources. usgs-runoff is the worked case:
rainfall at Cppt = 18 mg/L is the only pollutant source, the aquifers draw
up to ~2,000 cfs back out of the receptor nodes, and legacy's node
concentrations climb to 47 mg/L while v6 plateaued below 18 — every
subcatchment series bit-exact, every runoff-quality ledger equal to the
third decimal, and the node concentrations 0.34-0.38 of legacy's. P001,
whose groundwater never withdraws, matched exactly at 18.000 throughout,
which is what pointed at the denominator.
The fix keeps the per-source loaders as they are and corrects the LATERAL
share of the denominator afterwards to legacy's floored net, taken from the
flow side's own nodes.lat_flow — the same newLatFlow legacy floors. It is
GATED on a nonzero negative-component accumulator (aquifer-ward GW, negative
external, DWF or interface inflows), so a deck with no withdrawing lateral
takes a correction of EXACTLY ZERO and keeps its bytes; the S3 2D coupling
volume stays outside the correction (legacy has no 2D seam).
usgs-runoff FAIL (1,470 cells after
5d0f96f) -> PASS, bit-exact
2015-simple-yet-interesting-network-
infoswmm-13 FAIL (38,206) -> PASS, bit-exact
full_nolids repro FAIL (1,470) -> bit-exact
With 5d0f96f this closes usgs-runoff completely: 7,570 cells at the start
of the round, zero now.
Verified over the reachable set: the correction can only fire on a deck with
pollutants AND a negative lateral component, and two-way groundwater is the
only corpus source of one — all 17 pollutant+groundwater decks re-run, plus
a 15-deck sanity slice of ordinary quality PASS decks that must stay
byte-identical (they do; the gate makes their correction exactly zero).
ctest in build/gate: the four standing reds (ard_node_store, msx_parity,
dw_unsteady_friction, xsect_parity) plus test_engine_inp_writer_atomic,
which fails at clean HEAD and is not from this commit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… as legacy's does
legacy lid_addDrainInflow weights BOTH halves of the drain's delivery by
where the routing instant falls between the two runoff steps (lid.c):
q = (1-f)*oldDrainFlow + f*newDrainFlow
w = (1-f)*oldDrainFlow*oldQual[p] + f*newDrainFlow*newQual[p]
v6 filled per-node accumulators once per runoff step and consumed them as a
CONSTANT rate through that step's routing substeps. On a smooth drain the
difference hides below tolerance; on a sharp first flush it is a full
runoff step of lead: swmm-5-model-with-lid-but-no-bottom-infiltration's
node XXXXX00035 read 899 mg/L of SF1 at report period 8 where legacy reads
~0, with legacy's 999 mg/L arriving at period 9.
The per-node accumulators hold exactly legacy's products (drain_cfs x
subcatchment conc, summed over units), so the fix is the missing OLD pair:
the runoff-step clear now rolls the outgoing rates into
lid_drain_qual_load_old / lid_drain_qual_vol_old before zeroing, and the
routing-step loader interpolates both with the same runoff_interp_f
addWetWeatherLoads already uses.
swmm-5-model-with-lid-but-no-bottom-infiltration
FAIL (246 cells after f38f625; 7,172 at the round's start)
-> PASS, bit-exact
DELIBERATELY NOT TOUCHED: the drain's WATER channel
(nodes.lid_drain_inflow, consumed by assembleLateralInflows) still applies
the current rate unweighted. Legacy interpolates that half too, but every
LID deck's hydraulics are bit-exact today with the constant form — the
drain flows involved sit below the comparator's tolerance — and aligning it
would move hydraulic ULPs across the whole LID corpus for no verdict. The
quality pair uses its own interpolated volume, as legacy's w/q pairing
requires. Recorded here so the asymmetry is a decision, not an oversight.
The age/temperature accumulators beside the drain keep their current-rate
form as well (no legacy counterpart to match).
Verified over the LID + groundwater-quality regression set (80 decks: every
scored deck with LID usage, every pollutant+groundwater deck, and the
byte-identity sanity slice): usgs-runoff and 2015-simple-yet-interesting
hold their PASSes, all previously-PASSing decks still bit-exact, ZERO
regressions.
ctest in build/gate: the four standing reds (ard_node_store, msx_parity,
dw_unsteady_friction, xsect_parity) plus test_engine_inp_writer_atomic,
which fails at clean HEAD and is not from this commit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This branch was successfully deployed
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.