Merge development into ngwpc-candidate for release 3.1.2.4.0-rc1 - #122
Merged
Conversation
…s selected to represent fp outflows
… feature/levelpool
Use new EWTS `configure_existing_logger`
Merge ngwpc-candidate into development for release 3.1.2.3.0-rc2
eloss_sub was only assigned in the by-subnetwork-jit-clustered branch (via eloss_df.loc[...], which itself raised KeyError when eloss_df is empty). The serial, by-network, by-subnetwork-jit and bmi branches referenced eloss_sub without ever assigning it, raising UnboundLocalError whenever those parallel_compute_method values were used. Assign eloss_sub consistently in all five branches with eloss_df.reindex(param_df_sub.index).fillna(0.0), which is robust to an empty or partly-missing eloss_df and also resolves the original V3 NHD empty-eloss KeyError. Behaviour-preserving for a valid eloss_df.
Replace the per-reach `set(reaches) & wbodies_segs` (which allocates a new set and materialises the full intersection for every reach) with `wbodies_segs.isdisjoint(reaches)`, which short-circuits at the first shared element and allocates nothing, and add an early return when there are no waterbody break segments. The function is called per cluster per order in the parallel dispatch; ~13% end-to-end speedup on the CONUS NHF run with output unchanged.
qdpp was never declared in the cdef block and the qlat_add_loc
if/elif/elif chain has no else branch, so both qlat and qdpp could be read
uninitialized (-Wmaybe-uninitialized). Declare qdpp and zero-initialise
qlat/qdpp at the top of the per-reach loop. A no-op for qlat_add_loc in
{0,1,2} (overwritten before use) and merely deterministic otherwise;
verified bit-identical to baseline.
…hannel params The Fortran kernel `muskingcungenwm` previously had a dead guard (`.le. 0.0_prec` with a commented-out body), which under IEEE 754 returns false for NaN — so NaN inputs silently propagated through `sqrt(s0)` into NaN velocity in the routing output. Two-layer fix: 1. Fortran kernel (defense-in-depth): add `ieee_is_nan` checks alongside `.le. 0.0_prec` for n, s0, z, bw; `error stop` with a descriptive message on bad input (was: silently fell through). 2. Python preprocess (fail-fast): validate channel-parameter columns in `flowpaths` for finiteness during `read_geo_file`; raise `ValueError` listing affected columns and fp_ids before any routing runs.
…arning When a divide's percentage_area_contribution does not sum to 100, the shortfall used to be spread evenly over its virtual flowpaths so the divide's whole direct runoff was routed. The waterbody-on-vfp change replaced that with a warning, so an 80% divide silently loses a fifth of its lateral inflow, everywhere downstream, for the entire run. Redistribute again, exempting forced-routing headwaters: their temporary div holds a single vfp with a partial share that is applied against the ORIGINAL div's runoff once div_id is mapped back, so topping it up to 1.0 would hand the headwater the whole divide on top of what its siblings already take. The warning keeps the same exemption.
fix: prevent temporary fp_id's from being included in stream output
distutils is gone in Python 3.12, so the Cython extension builds now go through setuptools. Dependency pins are refreshed to the versions the build and CI actually run against.
Newer xarray no longer decodes some NWM time and coordinate variables the way these readers assumed, which surfaced as wrong dtypes rather than an error. The readers now state what they expect and convert explicitly.
Internal calls were using pandas and numpy idioms that are already deprecated, so a clean run buried real warnings under our own noise.
…drofabric versions Lake and hydrolocation layers changed column names between hydrofabric releases, and the old lookups silently produced empty frames instead of failing. Resolution now accepts the known spellings and raises when none of them is present.
compute_network_structured reads several arrays positionally and nothing validated them, so a mismatch produced plausible wrong numbers instead of an error. Observations are aligned to model steps once at the compute boundary, channel loss defaults to zeros rather than raising, and the Courant output buffer is widened to the six columns the kernel writes.
The area-scaling correction needs contributing drainage area per segment and a VPU id to resolve its regional exponent. Both are carried through discretization onto the routed network, with layer validation so a hydrofabric missing them fails at load.
Adds the streamflow_scaling block: the area-scaling exponent with its per-VPU overrides, the travel-time horizon and its wave celerity, the flow floor guarding the confluence split, and the spread memory control. Every failure mode this DA has is a silent one, so the guards reject at parse time rather than warn. Scaling is NHF only and mutually exclusive with nudging, which drives the same override. The horizon and celerity must be finite and positive: a bare positive bound admits infinity, which degenerates the lag instead of failing.
Builds one upstream tree per gage at network construction, walking upstream and stopping at other DA gages and at waterbodies so the trees are disjoint. Pruned branches stay in the confluence split denominator, so a stopped branch withholds its share instead of handing it to a sibling. The trees depend only on topology and the network's gage set, which keeps the cached execution plan valid across forcing windows. Replaces the standalone extraction script.
Spreads a gage innovation over its tree: area scaling along linear steps, flow-ratio split at confluences, clamped so the correction can never amplify as it moves away from the observation. A segment tau upstream receives the correction shifted by tau, and a segment beyond the horizon receives nothing, which is the reach limit. Past the last observation the edge persists the latest increment and decays it on the same clock as every other stale observation here. Ships as a Cython kernel with a NumPy reference and a 2000-case fuzz oracle asserting bit-level agreement, since the compiled path runs with bounds checking off and a shape mismatch there reads out of bounds rather than raising.
Injects accepted gage observations into the Muskingum-Cunge override before routing so the correction travels downstream through the router, then spreads the recorded innovation upstream after routing. Observations come from the shared TimeSlice reader, so this path sees the same files, QC gate and interpolation as nudging and reservoir persistence. A window's backward shift reads past its own end, so a non-final window defers its spread until the next window supplies that innovation as a halo, and forcing windows are enlarged when needed to cover the horizon. A partitioning that cannot satisfy that is rejected at run start rather than silently falling back to persistence. The realized window is written back alongside the configured one, since the two can differ.
Mirrors the CLI path so ngen gets the same correction, including the deferred-window halo, scoped within a single update call: non-final internal windows wait for the next window's innovation, and each update's final window closes with decayed persistence. Nothing is left pending across a checkpoint. The cycling warmstate is taken before the upstream spread, and the corrected hand-off is recorded separately, so a forecast branching from a checkpoint inherits the correction without debiting the next window's innovation.
run_bmi gains chunked stepping so the BMI path can be driven the way ngen drives it, one update per chunk, and the integration helpers carry the scaling DA configuration through.
…ldouts The benchmark directory had grown a flat mix of drivers, configs, Dockerfiles and one-off comparison scripts. Files move into folders by role, with no change to what any driver measures.
The notebook and runtime images duplicated most of the dev image and drifted from it. They become stages of Dockerfile.dev so there is one place to update the toolchain and pins.
travel_time_lag replaces lag_direction and lag_source and defaults false on the measured record; innovation_spread_h defaults 0. Finite-value guards on theta, min_flow_cms, and synthetic_obs_factor, and a validator requires synthetic_obs_baseline whenever the factor is set.
Explicit qlat_forcing_sets must cover lag_window_h plus innovation_spread_h when the run could supply it; a short final remainder folds into the window before it. Gages crosswalked to lake ids are excluded loudly (reservoir DA owns those segments), dropped pruned branches warn, and per-tree theta ships as a validated CSV.
Remove the NumPy fallback, the method parameter, and the area_scaling ablation; a lag shape the compiled kernel cannot express now raises. The equivalence fuzz retires with the reference it validated, and the pruned-confluence and lag-edge behaviors are restated as compiled-path tests.
Trace min(cn, 1) to match the solver's one-timestep K floor, fixing the 2.4x tau underestimate (OSSE median error 3.00 h to 2.00 h). Unresolved segments are counted by cause, a run that cannot trace fails closed, and the trace serializes with a hashed identity for checkpoints. The state hand-off instant always seeds untimed: the lagged read there falls past the analysis edge, and with the untimed seed the forecast is identical with the lag on or off.
create_state carries the trace with its identity and load_state restores it only on a match, clearing stale caches otherwise. The run window memory estimate drops to the measured 100 bytes per link timestep, and the window-size error names max_loop_size as a cause alongside the DA span.
METHOD.md gives the journal-style description including operational context; TIMING_DECISION.md and DELIVERY_HANDOFF.md carry the measured record through the default decision and the seed experiments; a runnable chain demo reproduces the arithmetic and plots it.
_refactor_reservoirs absorbed only the outlet virtual_fp_id declared by the lakes layer, so the other flowpaths crossing a lake polygon kept routing as Muskingum-Cunge channel inside the reservoir. On CONUS a lake spans 8.5 flowpaths, so roughly 7.5 of every 8.5 were modeled as channel. Consume the NHF lake_vfp_crosswalk layer (optional, so older geopackages keep the previous behavior) and absorb every crosswalked flowpath draining to the lake outlet. Lakes sharing a flowpath are clustered so none is claimed twice, then split back per lake by nearest outlet: a lake never moves its discharge point, serial lakes chain through topology rather than row order, and nothing below the outlet is absorbed. Many inlets are now normal, so only the outlet must be unique. Every absorbed flowpath reports the lake's release, which moves the peak at in-lake and downstream features. Updates the patuxent and conecuh bounds accordingly.
hy_id is the only key tying a gage row to its hydrolocation, and crosswalk_nex_flowpath_poi joins on it; site_no and gid both repeat, so neither can stand in. nhf 1.2.3 dropped the column, which surfaced as a KeyError raised deep inside a pandas merge. Add REQUIRED_COLUMNS for layers read in full, which name no column list for the existing check to use. Enforced only when the layer is present, so a gage-free domain stays valid.
The method section, delivery record, timing log, figures, and demo move to the t-route_dev workspace; the branch carries code and tests only.
Simple-scaling streamflow data assimilation with NHF lake subnetwork collapse
The pin lives in pixi.toml, src/troute-config/pyproject.toml, and src/troute-rnr/pyproject.toml, which have to agree. pydantic-xml 2.16.0 requires only pydantic>=2.6.0, so it needs no companion bump.
build: pin pydantic to ~=2.12.0
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
7 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.
No description provided.