pwnlayers3: physical-correctness fixes (botm/thickness/offshore)#34
Open
pwnlayers3: physical-correctness fixes (botm/thickness/offshore)#34
Conversation
Replace direct .values assignment with xr.apply_ufunc(dask="parallelized") in fix_missings_botms_and_min_layer_thickness so the monotonic-decrease invariant survives a future dask-backed array (the .values assignment silently no-ops on dask arrays). Applied identically in both pwnlayers and pwnlayers3 copies.
The pre-merge PWN dataset is validated for non-negative thickness, but the merge with REGIS can introduce layer crossings. Validate the merged layer model after fix_missings_botms_and_min_layer_thickness to catch any remaining negative thicknesses.
Treat NaN thickness the same as zero thickness so kh/kv values do not silently become NaN inside a layer's own mask when adjacent layer boundaries differ (e.g. S21 vs W21). The mask now combines np.isclose(thickness, 0.0) with ~np.isfinite(thickness), and the log message reflects "zero or undefined thickness".
Move botm.where(isin_bounds) to before fix_missings_botms_and_min_layer_thickness so the downward ffill only operates on cells inside at least one layer's mask. After the fix, re-mask with any_layer_mask = isin_bounds.any(axis=0) to avoid synthetic fill-from-top outside all boundaries, then re-apply the per-layer isin_bounds mask to preserve the original NaN pattern.
Per Edinsi 3.1 (p.12), botm.geojson points extend west of the coastline, and the nearest-neighbor fallback in get_botm could extrapolate layer elevations into the seabed. The noordzee_clip polygon is now read inside get_botm and used to drop offshore cells from the per-layer boundary mask both before griddata interpolation and in the final clip, so offshore cells return NaN.
This reverts commit 31a3550.
Each split sublayer is now decided from its own mask_model_other and transition_model only. Removes three cross-layer leakage points so lower-layer transition rings track the actual per-layer extent rather than being inflated to upper-layer extents. - Drop fix_missings_botms_and_min_layer_thickness on the inputs and on the merged result in combine_two_layer_models. Validation already guarantees no NaN inside mask_model_other and a finite REGIS, so the cross-layer ffill+accumulate is no longer needed. - Replace the post-merge fixer with a per-column np.minimum.accumulate along the layer axis only — no ffill, no cross-cell movement. - Refactor _compute_thickness_ratios to take fallback="equal" and assign 1/N where mask_valid is False; remove the cross-layer nearest-neighbor spread inside _adjust_botm_with_nearest_ratios. - Rename _interpolate_ds/_interpolate_da to _interpolate_ds_inplace/ _interpolate_da_inplace, assert per-layer that isvalid and ismissing are disjoint and complementary on icell2d, and guard against an empty isvalid set. - Strengthen the no-NaN-inside-mask assertion to name the offending variable and point at upstream builders. - Drop the redundant post-merge fix_missings_botms_and_min_layer_thickness call in pwnlayers3/layers.py — the merge function now guarantees per-column monotonicity itself.
Co-authored-by: Copilot <[email protected]>
…ed surface water handling Co-authored-by: Copilot <[email protected]>
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
1 participant
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
Four narrow correctness fixes to
pwnlayers3/layers.py(and one topwnlayers/utils.py). No refactors, no new abstractions; each commit is a single targeted change.ebd2979) — Replaceout.values = np.minimum.accumulate(...)withxr.apply_ufunc(..., dask=\"parallelized\")infix_missings_botms_and_min_layer_thickness(bothpwnlayers3/layers.pyandpwnlayers/utils.py). Theout.values = ...form silently no-ops on dask-backed arrays; the apply_ufunc form preserves the monotonic-decrease invariant under both numpy and dask.ae48346) — Add a post-merge non-negative-thickness assertion inget_pwn_layer_modelmirroring the pre-merge check; the merge between PWN and REGIS can produce layer crossings that were previously smoothed silently.8b0b56c) — Make_guard_zero_thicknessNaN-aware:zero_d = np.isclose(t, 0.0) | ~np.isfinite(t), log message updated to "zero or undefined thickness". Stops NaN thickness leaking throughkh = KD/dinto kh/kv inside a layer's own mask.9670188) — Inget_botm, clipbotm.where(isin_bounds)beforefix_missings_...(rather than after); re-mask the post-fix result withany_layer_mask = isin_bounds.any(axis=0)and a final per-layer mask. Removes the structural cause of the NaN-thickness problem that Fix 3 defends against.Reverted
31a3550+50a7521revert) —noordzee_clipoffshore mask inget_botmwas reverted at the user's request; PWN data is preferred everywheremask_pwnis True, including offshore.Notes on overlap
8b0b56c) before merge if you want only the structural fix. Keeping both is safe.Test plan
09pwnmodel2/03_pwnlayers2026.py) and confirm noValueError(\"Merged layer thickness should be non-negative\")regression.isin_bounds[k]mask (was previously occurring at e.g. the S21/W21 boundary per Edinsi 4.1).