Merge development into ERF-Hazard - #371
Merged
Merged
Conversation
…rf-model#3962) (erf-model#3963) Above the boundary layer, both the explicit TKE update and the implicit TKE solve clamp TKE to exactly shoc::constants::min_tke(), so the bare "tke > min_tke()" test in diagnose_active_top() was settled by the last bit of the tridiagonal solution's exponentially decaying tail. Host and device builds differ there by one ulp, since the CUDA build contracts to FMA (--use_fast_math is on by default in the GNUmake path) and the host build does not. That flipped the diagnosed active top from step to step, changing both the magnitude of delta_tabs and the depth over which the energy fixer spreads it. For the GABLS1 ABL case with erf.pbl_type=NATIVE_SHOC this made a CUDA run and a pure-CPU run differ by ~1e-5 relative in velocity after 50 steps, against ~1e-10 for the same comparison with MYNN25. Compare against a threshold carrying a small relative margin above the floor instead. The margin is precision-aware so it stays well clear of one ulp in single-precision builds, and both the Array4 and Vector overloads of diagnose_active_top() use it so the device path and the unit-tested host path cannot diverge. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…r uses (erf-model#3961) * Keep the MRF scheme from raising invalid floating-point flags it never uses Every MRF run died at its first step under amrex.fpe_trap_invalid in an optimised build, on a neutral ABL with nothing wrong in the results, while a debug build ran clean. The cause is the optimiser, not the physics: three guarded expressions in ComputeDiffusivityMRF were being evaluated speculatively, ahead of their guards, with arguments that make them invalid, and the discarded NaN still raised the flag the trap watches. 1. pbl_blend_factor computed (dx/L)^2/(1+(dx/L)^2) behind an early return for L <= 0. The expression is loop-invariant, so it was hoisted out of the K loop above the caller's L > 0 test; with blending off L is 0 and dx/L is inf, inf/inf is NaN. It is now dx^2/(L^2+dx^2), the same function with a denominator of at least dx^2 for every L, and exactly 1 when L <= 0. Guarding the division with a select does not help, the division gets folded into the select's arms. 2. The unstable stability functions take sqrt(max(-Ri, 0)) inside the Ri <= 0 arm. Knowing Ri <= 0 there, the optimiser removed the max and then hoisted the bare sqrt above the selection, where Ri is positive. Writing min(Ri, 0) instead moved the problem: the sqrt was folded into the arms of the min. The argument is now sqrt(|Ri|), evaluated before the selection; it equals sqrt(-Ri) in the arm that uses it and has nothing to fold. 3. The unstable phi functions raise max(1 - 16 HOL, 0.01) to a negative power inside the L < 0 arm, the same construct, fixed the same way: the base is 1 + 16 |HOL|, equal to 1 - 16 HOL in that arm and at least 1 everywhere. Results are unchanged: with the trap off the neutral ABL of the report is bit-identical to the previous code at step 10 with blending off and on, and the MRF_Enhancements/blending_active canonical is bit-identical at step 5. With the trap on the neutral case now runs 200 steps on four ranks, and stable and unstable variants (surface heating of -3 and +3 K/h) run 60 steps, where before the first step aborted. The debugger confirmed each site from the faulting instruction and its registers (a 200 m dx divided by a zero L, then the sqrt of a negative Ri). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Tighten the FPE-trap comments: single-spaced, one line in the blending header Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…constant (erf-model#3962) (erf-model#3964) clip_third_moments() replaced an out-of-range w3 with shoc_w3clipdef(), a fixed positive 0.02, following E3SM/EAMxx. That has two problems. It discards the sign of the third moment, so a strongly negatively skewed column is handed back a positively skewed w3, which flips the direction of the skewness that the assumed PDF and the third-moment closure then see. It also ignores the bound the clip is meant to enforce: when clip_cond = 1.2*sqrt(2*w_sec^3) is smaller than 0.02, the "clip" moves |w3| further outside the bound rather than back inside it, and in either direction it is a large discontinuous jump in w3. Clamp the magnitude to clip_cond and keep the sign instead. That is continuous in w3, always lands on the bound, and leaves in-range values untouched. This is the second item in the "Related problems" section of erf-model#3962. It is latent for the cases in the test suite: instrumenting the branch shows it never fires in any of the twelve SHOC regression cases, and the plotfile comparisons are unchanged. It matters for moist or convective columns where the third-moment path carries a large skewness. The ERF-native SHOC is deliberately not a bit-for-bit port of EAMxx SHOC (Source/PBL/Shoc/README.md), so this diverges from upstream on purpose. Unit tests cover the sign-preserving limit in both directions, an in-range value that must be left alone, and a quiescent level where w_sec is zero and w3 must therefore go to zero rather than to 0.02. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nge (erf-model#3963) (erf-model#3966) erf-model#3963 moved the SHOC energy fixer's active-top test off the exact min_tke() floor and onto a threshold carrying a small relative margin. That shifts the diagnosed top by a level in columns whose TKE sits at the floor, which changes both the magnitude of delta_tabs and the depth it is spread over, so the cloudy SHOC answers moved slightly. The gold files were not regenerated at the time and the five cloudy cases have been failing since. The differences are small and are of exactly the size the energy fixer itself carries: for the unstable cloud cases theta is off by 1.2e-5 K against a 9.1e-6 allowance and density by 3.8e-8 relative against 3.4e-8; the stable cloud case moves a similar amount across more fields. Nothing here is a new physical signal, and the property checks that run ahead of the gold comparison pass throughout. Confirmed the code change is the cause rather than a compiler or machine difference: with the pre-erf-model#3963 threshold restored, this machine reproduces the existing gold files and all twelve SHOC cases pass. The clear-sky cases (SHOC_Stable_Clear, SHOC_Unstable_Clear_BOMEX), which compare with strict fcompare at 2e-10, are unaffected and their golds are untouched. Regenerated plt00020 for SHOC_Stable_Cloud, SHOC_Unstable_Cloud_SatAdj, SHOC_Unstable_Cloud_NoCond, SHOC_Unstable_Cloud_Kessler, and SHOC_Unstable_Cloud_WSM6 from development at f63fc5d. The full regression label now passes, 99 of 99. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-model#3965) * Keep the moist HSE initialisation off the invalid-operation trap The MoistBubble regression test arms amrex.fpe_trap_invalid and dies in Problem::erf_init_dens_hse_moist in optimised builds (SIGILL on macOS arm64, clean at -O0). compute_theta is called only when T_from_theta is set, but it is inlined into the Newton loops and clang evaluates its arm speculatively; with the placeholder values of zero passed for theta_0, theta_tr and T_tr that arm forms g/(Cp_d*0) and then 0*exp(inf), which raises the invalid flag even though the result is discarded. The placeholders are now physical temperatures (300 K) in the helper's default arguments and in erf_init_dens_hse_moist, so every speculated expression stays finite. A selected result never reads them: MoistBubble passes with the trap armed and its plotfile is bit-identical to the pre-fix binary run with the trap disabled (fcompare at zero tolerance); SquallLine_2D and SuperCell_3D, which pass real values, still match their gold files. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Tighten the placeholder comments Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
* Remove native SHOC host diffusion modes * Add tests for native SHOC host diffusion removal * Update Native SHOC documentation
* PBLH Corrections * Space * Space * YSUNew * YSUNew * Implement Vogelezang & Holtslag (1996) shear-correction for MRF and YSUNew PBL schemes Co-authored-by: hgopalan <1108371+hgopalan@users.noreply.github.com> * Add PBLH spatial smoothing feature for both MRF and YSUNew PBL schemes Co-authored-by: hgopalan <1108371+hgopalan@users.noreply.github.com> * Smoothing should be model independent * QNSE * Fix QNSE turbChoice * QNSE branch proper * QNSE Test Case * Fix * Fix zero wind speed MOST * Revert unintended submodule update * YSUNew * Scale with PBLH * Counter Gradient Comments * Fix Implicit Diffusion Signs * Update signs * Fix the denominator * Added RegTest and fixed a compile bug * Added RegTest and fixed a compile bug --------- Co-authored-by: Ann Almgren <asalmgren@lbl.gov> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: hgopalan <1108371+hgopalan@users.noreply.github.com>
Co-authored-by: Aaron M. Lattanzi <103702284+AMLattanzi@users.noreply.github.com>
* fix LSM construction with remakelevel. * Fix dz_min location.
* Variable roughness fixes and Donelan. * update abl_most_sst. * Update z0 with lsm as well. * Fix the Windows and NetCDF unit-test CI failures (erf-model#3953) * Make ABL_MOST_SFC/SST perturbations decomposition-invariant * Add decomposition-invariant index-keyed perturbation RNG * Address code review findings on ABL perturbation work * Fix WPS map projection in TerrainNetCDF test fixture * Revert "Fix the Windows and NetCDF unit-test CI failures (erf-model#3953)" This reverts commit 1737bc3. * update abl_most_sst again. * remake benchmark without vert implicit. * Fix stale values from limiter in iteration. * Fix theta_v^* for olen calc. * Address review. * fix mpi on windows. --------- Co-authored-by: Jean M. Sexton <jmsexton@lbl.gov> Co-authored-by: Ann Almgren <asalmgren@lbl.gov>
Brings in the ten upstream commits since the release branch point: the Donelan and variable-roughness MOST rework (erf-model#3952), PBLH corrections with the QNSE stable stability functions and the VH96 shear correction (erf-model#3486), RemakeLevel asymmetries (erf-model#3969), the removal of the native SHOC host_diffusion runtime modes (erf-model#3967), the SHOC third-moment and energy-fixer fixes with their gold files (erf-model#3962-erf-model#3966), the WRF boundary moisture GPU unit-test allocation (erf-model#3968), and the two fixes that started on this branch and went upstream, the MRF floating-point flags (erf-model#3961) and the moist HSE initialisation (erf-model#3965). Ten files were touched on both sides and three conflicted. Both sides are kept everywhere they are independent: - ERF_TurbStruct.H: the fire and dust MRF parameters of this branch next to the QNSE and VH96 members from upstream, in both the YSU and the MRF branch of the ParmParse block. - ERF_ComputeDiffusivityMRF.cpp: the QNSE stable stability functions are taken from upstream, but the unstable arm keeps this branch's pow(1 + 16|HOL|) form and the sqrt(|Ri|) form. Those shapes are what keep the scheme off the invalid-operation flag under amrex.fpe_trap_invalid (erf-model#3961, Hazard #349); upstream's max(1 - 16 HOL, 0.01) clamp is equivalent in the arm where it is used, so nothing is lost. The QNSE stable K-profile condition is taken as upstream writes it, next to this branch's fire-boosted wstar comment. The dust Schmidt-number scaling stays alongside upstream's note on the HGAMT/HGAMQ units convention. - The PBLH write-back to SurfaceLayer is upstream's set_pblh(level, pblh_mf), which fills the field inside the kernel instead of the host-side copy this branch carried, and is the GPU-clean form. The guard and the reason the dust layer needs it are kept as a comment. - ERF_PBLScaleAwareBlending.H: same code on both sides, this branch's fuller comment kept. Verified: erf_exec and erf_unit_tests build with no errors and no warnings outside the submodules, all 471 unit tests pass (454 before, upstream adds 17 SHOC cases), and upstream's inputs_arctic_qnse deck, which exercises the hand-merged MRF code, runs clean under amrex.fpe_trap_invalid. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 of 4 tasks
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
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
Merges
developmentintoERF-Hazard: the ten upstream commits since the release branch point. Both sides are kept everywhere they are independent.ERF_MOSTStress.Hrewritten,ERF_MOSTUtils.Hadded,ERF_MOSTRoughness.Hremoved. No ERF-Hazard code referenced the removed header.host_diffusionruntime modes (Remove Native SHOC host_diffusion runtime modes erf-model/ERF#3967), the WRF boundary moisture GPU unit-test allocation (Fix WRF boundary moisture GPU unit-test allocation erf-model/ERF#3968).Conflicts
Ten files were touched on both sides; three conflicted.
ERF_TurbStruct.HERF_PBLScaleAwareBlending.HERF_ComputeDiffusivityMRF.cppIn
ERF_ComputeDiffusivityMRF.cppthe QNSE stable stability functions are taken from upstream, but the unstable arm keeps this branch'spow(1 + 16|HOL|)andsqrt(|Ri|)forms. Those shapes are what keep the scheme off the invalid-operation flag underamrex.fpe_trap_invalid(erf-model#3961, Hazard #349); upstream'smax(1 - 16 HOL, 0.01)clamp is mathematically equivalent in the arm where it is used, so nothing is lost either way. The QNSE stable K-profile condition is taken as upstream writes it, next to this branch's fire-boostedwstarcomment, and the dust Schmidt-number scaling stays alongside upstream's note on the HGAMT/HGAMQ units convention.The PBLH write-back to
SurfaceLayeris upstream'sset_pblh(level, pblh_mf), which fills the field inside the kernel rather than the host-side copy this branch carried, and is the GPU-clean form; the guard and the reason the dust layer needs it are kept as a comment.Test plan
erf_execanderf_unit_testsbuild with no errors and no warnings outside the submodulesinputs_arctic_qnse, which exercises the hand-merged MRF code, runs clean underamrex.fpe_trap_invalid=1 amrex.fpe_trap_zero=1FireRestart: seven rows restart exactly, numbers unchanged from before the mergeFireRosComparison: 30 variants, identities hold, numbers unchangedFireNearWall(four ranks): numbers unchanged🤖 Generated with Claude Code