CRTM v3.2.0 release: integrate feature/btj_REL-3.2.0 - #362
Draft
BenjaminTJohnson wants to merge 269 commits into
Draft
CRTM v3.2.0 release: integrate feature/btj_REL-3.2.0#362BenjaminTJohnson wants to merge 269 commits into
BenjaminTJohnson wants to merge 269 commits into
Conversation
Since fe3bf46 the default LUT format is NetCDF and the .bin AerosolCoeff symlinks are no longer staged in build/test/testinput. The three Unit_AerosolScatter tests still hardcoded 'AerosolCoeff.GOCART-GEOS5.bin' and were failing on file-not-found. Switch each to 'AerosolCoeff.GOCART-GEOS5.nc4' with netCDF=.TRUE.
Inside the OMP-parallel channel-thread loop (DO nt = 1, n_channel_threads), the warning WRITE for Obs_4_downward used the unindexed RTV%Obs_4_downward%idx, which is an INTEGER array (one per allocated RTV element) rather than a scalar. That made Atm%Level_Pressure(...) a REAL array, breaking the WRITE which expects scalar arguments and producing the Fortran runtime error: Expected INTEGER for item 4 in formatted transfer, got REAL at line 816, but only when n_channel_threads > 1. The result was that all test_forward_Downwelling_Radiance_* tests failed at OMP_NUM_THREADS >= 4 and passed at <= 2 (where n_channel_threads is forced to 1). Use RTV(nt)%... to match the surrounding code (line 811, and the parallel aircraft-pressure block above this one).
Three latent bugs in the channel-thread parallel region surfaced as
heap corruption / SEGFAULTs at OMP_NUM_THREADS >= 4 in the Forward
model (and at >= 19 in TL). Root causes:
1. Unindexed RTV/RTV_Clear flag broadcasts inside the channel-thread
loop in CRTM_Forward_Module:
RTV%Solar_Flag_true = .TRUE. ! all elements
RTV_Clear%Solar_Flag_true = .TRUE.
RTV%Visible_Flag_true = .TRUE.
Concurrent threads wrote to every per-thread element, mixing flag
state across channels and corrupting downstream scratch buffers.
Same class as the prior Obs_4_downward fix (bbdf187). Replaced with
RTV(nt)%... / RTV_Clear(nt)%... .
2. Off-by-one in the n_inactive_channels chunk-bucket mapping:
nt = FLOOR(REAL(l)/REAL(chunk_ch)) + 1
misclassifies boundary channel l = k*chunk_ch into chunk k+1.
With inactive boundary channels (e.g. test_ChannelSubset selecting
every 100th of 8461 IASI channels), the cumulative shift left
ic(k+1) one too low, so ln_k_start was one too high, and the last
thread eventually wrote RTSolution(n_Channels+1, m) -- out-of-bounds
heap write. Replaced with nt = (l-1)/chunk_ch + 1, clamped to
n_channel_threads.
3. Missing clamp on non-last end_ch:
end_ch = start_ch + chunk_ch - 1
could exceed n_sensor_channels when n_channel_threads*chunk_ch
overshoots (e.g. ABI/16ch at OMP=7: nt=6 had start_ch=16, end_ch=18,
reading Process_Channel(17..18) out-of-bounds). Clamped via
MIN(..., n_sensor_channels).
Fixes #2 and #3 applied in CRTM_Forward_Module, CRTM_Tangent_Linear_Module,
and CRTM_K_Matrix_Module for structural consistency. (K's channel-thread
path is currently bypassed by the issue-#231 preprocessor block, so the
K change is precautionary.) Fix #1 is Forward-only; TL/K do not have the
unindexed-broadcast pattern, and Adjoint uses scalar RTV.
Verification: ctest sweep (excluding the two unrelated PARMIO failures
noted previously) is now 193/193 at OMP_NUM_THREADS in
{unset, "", 1, 2, 4, 8, 19}, up from 192/193 at OMP=4, 191/193 at OMP=8,
and 183/193 at OMP=19.
Re-enables OpenMP-over-channels for the K-matrix path on every compiler except legacy classic ifort (icc/ifort, pre-LLVM), which lacks local toolchain coverage and stays on the serial fallback. Two changes are required to do this safely; either alone is insufficient. 1) Re-apply the per-channel NLTE adjoint init from 23f37aa (lost in intervening merges/reverts). At the start of each Channel_Loop iteration: IF ( Opt%Apply_NLTE_Correction .AND. NLTE_Predictor_IsActive(NLTE_Predictor) ) THEN NLTE_Predictor_K(nt) = NLTE_Predictor NLTE_Predictor_K(nt)%Tm = ZERO NLTE_Predictor_K(nt)%Predictor = ZERO END IF Without this, NLTE_Predictor_K(nt) carries state from the previous channel within the same thread. The serial reference data was produced channel-by-channel through the full sensor channel list, so any per-thread channel partition that differs from the serial order produces different residual state and different Atmosphere_K Jacobians. Symptom under channel-thread parallelism without the fix: K-matrix tests pass at OMP_NUM_THREADS in {1, 2, 4} and fail at OMP_NUM_THREADS >= 8 with "Atmosphere_K Jacobians are different!" while RTSolution_K is correct. 2) Replace the three coarse `#if 1` / `#if 0` bypass blocks (introduced originally in 2e4a28d, broadened in bb9991b) with a compiler-conditional gate that bypasses only legacy classic ifort: #if defined(__INTEL_COMPILER) && !defined(__INTEL_LLVM_COMPILER) gfortran 13.x and ifx 2026.0.0 are now verified clean (sweeps below). Legacy ifort is left bypassed because we have no installation here to verify it; can be re-enabled in a follow-up if/when validated. Verification ------------ Standard ctest sweep (excluding the unrelated PARMIO_RC and PARMIO_FASTEM_DeltaSweep_AWS tests, which require missing inputs): gfortran 13.x, default build/, OMP_NUM_THREADS in {unset, "", 1, 2, 4, 8, 19} -> 193/193 every level. ifx 2026.0.0 (separate worktree, ifx-built netcdf at /usr/local/netcdf-{c,fortran}, parallel HDF5 from /home/ben/libraries/hdf5/build-parallel-icxifx), bypass-lifted + NLTE fix, OMP_NUM_THREADS in {1, 2, 4, 8, 19} -> 190/190 every level. Pre-fix (bypass lifted but NLTE init missing) failed at OMP=8 with test_k_matrix_ChannelSubset_iasi_metop-b and test_k_matrix_User_Emissivity_cris399_npp; OMP=19 added three more cris399 K-matrix tests with the same Atmosphere_K signature. All clean after the NLTE fix. JEDI risk --------- The bypass arose partly in response to fv3-jedi sigbus reports (886ad35 "attempt to address sigbus issues in fv3-jedi", c34c19b "welp, back to removing all openMP directives") that local CRTM ctest never reproduced. Local sweeps under both compilers are now clean, but a JEDI-side smoke test of the same workload that drove 886ad35 is still recommended before broader release. If JEDI surfaces the issue again, extending the gate to the responsible compiler is straightforward.
…ual gate
Test commands now reference only paths inside the build tree.
* CMake configure-time staging: PARMIO LUT and AWS Spc/TauCoeff are
symlinked into test_data/.../fix/{EmisCoeff/MW_Water,SpcCoeff,
TauCoeff/ODPS}/netCDF/ via the configurable PARMIO_LUT_SRC and
AWS_COEFF_SRC cache vars, then flow through the existing
crtm_test_input symlinks into testinput/. CTest commands and
Fortran defaults reference testinput/ only.
* test_PARMIO_RC_Residual demoted to build-only diagnostic. The
driver compares CRTM Kirchhoff*Mod sky reflection against PARMIO
Tb.f's specular Rvv0; the residual is a known structural mismatch
between the two atmospheric conventions (CRTM is the more physical
one), not a defect on either side. Source header documents this.
* Tests register conditionally on artifact presence, so upstream
builds without locally-generated PARMIO/AWS coefficients still
configure cleanly with a STATUS message.
The new fix_REL-3.2.0.0 tarball renames every formerly-.nc4 NetCDF
coefficient to .nc (NetCDF is the canonical format from REL-3.2.0;
no functional change to the file contents). Update every reference
in the test tree and the library defaults so tests find their
coefficients without relying on extension-fallback shims.
* src/CRTM_LifeCycle.f90: Default_{Aerosol,Cloud,IR*,VIS*}Coeff_File
defaults updated. CRTM_Init callers that don't pass explicit
filenames now look for .nc files directly.
* test/CMakeLists.txt: tarball name, checksum, and crtm_test_input
symlink list updated to fix_REL-3.2.0.0 and .nc names. (Part of
the same migration; staged alongside the source updates.)
* Twelve test sources under test/mains/{regression,unit}/ updated
via mechanical .nc4 -> .nc replacement for hardcoded coefficient
filenames (AerosolCoeff, CloudCoeff, IR/VIS EmisCoeff defaults,
and the GOCART-GEOS5 new/old fallback in test_AOD).
The test/testinput/single_profile.yaml file retains .nc4 references
to amsua_n19 obs/geoval files; those belong to a separate JEDI test
fixture and are not in the CRTM coefficient tarball.
SSU forward/k_matrix tests fail under fix_REL-3.2.0.0 because the
new tarball ships no SSU TauCoeff file (neither netCDF nor binary);
unrelated to this change.
The fix_REL-3.2.0.0 tarball is netCDF-canonical but shipped no SSU TauCoeff
in any format because the generic ODPS BIN2NC converters do not handle
ODSSU's per-cell-pressure container. Add a flat ODSSU netCDF schema
(standard ODPS dims + trailing n_TC_CellPressures axis) plus a standalone
ODSSUBIN2NC executable, and route ODSSU loads through use_netCDF in
CRTM_TauCoeff. Re-point the test/CMakeLists.txt SSU symlinks at the new
.nc paths and update the tarball MD5 to the netCDF-bearing respin.
Restores test_forward/k_matrix_SSU_ssu_n{06,14}.
The PARMIO LUT and AWS Spc/TauCoeff are now staged inside the tarball tree at fix_REL-3.2.0.0/fix/..., so test_data discovery no longer needs to reach outside build/test_data/**. Drop the "production" infix from the LUT filename per the new convention. - test/CMakeLists.txt: replace the staging-from-outside block with a presence check at the canonical fix/ paths; update the LUT testinput symlink target and crtm_test_input list to PARMIO.MWwater.EmisCoeff.nc. - test/mains/regression/parmio_tlad/test_PARMIO_*.f90 (10 files): point every reference at './testinput/PARMIO.MWwater.EmisCoeff.nc' (was either '../parmio/Outputs/sweep/lut_production/...' or the older testinput path with the production infix).
…EL-3.2.0.0.tgz tarball.
The PARMIOCoeff RC group's Rdown LUT was rank 8 (pol, tau, foam, sss, sst, U10, theta, freq), which nvfortran rejects because it caps array rank at 7. Polarization is a 2-state switch that is also stored as separate Rdown_v / Rdown_h variables on disk, so split it in memory to match: each pol now has its own rank-7 array. The reader drops its 8-D staging buffer and writes directly into the new fields. Interp_Rdown / TL / AD dispatch on ipol to pick V or H. Also adds an NVHPC compiler-flags file (CRTM had none) and adjusts five unit tests that used the legacy module-level UnitTest_* generic forms to the type-bound (obj%X) form the source already labels as preferred -- nvfortran's generic resolver fails on the legacy form. Two convergence tests had REAL(16) ratios that nvfortran does not support; dropped to REAL(fp) since the comparison tolerance is 0.1. Verified: 194/194 ctest pass on gfortran, ifx 2025.3, nvfortran 24.11.
CRTM_Init now accepts an optional PARMIOCoeff_File argument that loads
the LUT via CRTM_PARMIOCoeff_Load. File_Path is prepended like the
other *Coeff_File arguments. CRTM_Destroy frees the LUT when it has
been loaded.
The MW_Water dispatcher's PARMIO branch is now guarded by both the
per-call flag and the load state: dispatch routes through PARMIO only
when Options%Use_PARMIO_Model is set AND CRTM_PARMIOCoeff_IsLoaded()
is true. If the flag is set without a loaded LUT, dispatch falls
through to FASTEM silently -- byte-identical to a pre-PARMIO build.
This applies to Forward, TL, and AD.
Adds test_PARMIO_Lifecycle (registered under PARMIO_LUT_PRESENT). One
ATMS scene exercises three contracts:
A. Init with PARMIOCoeff_File + flag on -> routes through PARMIO
B. Same init, flag off -> FASTEM baseline
C. Init without PARMIOCoeff_File, flag on -> byte-identical
silent FASTEM fallback
Observed:
- A vs B max |dT|: 1.5 K on ATMS (dispatch confirmed)
- C vs B max |dT|: 0.0 K (fallback contract holds)
- CRTM_PARMIOCoeff_IsLoaded() is .FALSE. after CRTM_Destroy
Full ctest suite: 195/195 pass on gfortran.
The three obs-space drivers (ATMS clear-ocean, AWS-1 smoke, GMI SOCA)
no longer perform a two-step init. PARMIOCoeff_File is now passed
directly to CRTM_Init, and the LUT is freed by CRTM_Destroy. The
redundant explicit CRTM_MWwaterCoeff_Load_FASTEM('FASTEM6') call is
also removed -- CRTM_Init loads FASTEM6 by default in its
Microwave_Sensor block.
This makes the drivers the canonical example for downstream callers
(UFO/JEDI, application code) on how to opt into PARMIO via the
public CRTM API.
Behaviour unchanged. All three drivers still build clean; the
registered PARMIO test suite (TLAD, Dispatcher, FASTEM_DeltaSweep,
FASTEM_DeltaSweep_AWS, Lifecycle) is green.
… prefer canonical coeff dirs The substructure-file lookup at the end of SpcCoeff_netCDF_ReadFile was declared CHARACTER(LEN(Filename)) :: sub_filename. The .SpcCoeff. -> .NLTECoeff. substitution is one character longer, so the trailing 'c' of '.nc' was silently truncated and File_Exists returned false. The NLTE sibling load quietly no-oped for every absolute-path caller, leaving SpcCoeff%NC default-initialized and producing several-K brightness-temperature drift on NLTE-active channels for all CrIS/AIRS/IASI sensors. ACCoeff was unaffected because 'ACCoeff' is shorter than 'SpcCoeff'. Buffer is now LEN(Filename)+16, with comments explaining why. Also teach the loader to honor the canonical fix-tree layout: when the SpcCoeff path contains /SpcCoeff/netCDF/, substitute that segment with /NLTECoeff/netCDF/ or /ACCoeff/netCDF/ to find the sibling. Falls back to same-directory sibling lookup for non-canonical (flat) layouts. Verified end-to-end: cris-fsr_n21, cris-fsr_n20, mhs_n19, and a flat temp-dir layout all load with the expected substructure associations.
After 695e54b the SpcCoeff netCDF reader actually loads the <sensor>.NLTECoeff.nc sibling. The regression drivers init with File_Path=./testinput/ (a flat dir), so the canonical /SpcCoeff/netCDF/ -> /NLTECoeff/netCDF/ substitution doesn't apply and the reader falls back to a same-directory sibling lookup. Only the NLTECoeffs explicitly listed in crtm_test_input get symlinked there, so cris-fsr_n21 -- exercised by ~10 regression/OMP tests -- was running with NLTE silently off. Add NLTECoeff/netCDF/cris-fsr_n21.NLTECoeff.nc to the symlink list; CREATE_SYMLINK_FILENAME stages by basename so it lands next to cris-fsr_n21.SpcCoeff.nc in testinput/. Verified: gcc tree ctest is 195/195 after re-baselining (cris399_npp no longer fails, cris-fsr_n21 now picks up its NLTE correction).
…nitializations - In ODCAPS modules: Remove `=> NULL()` pointer initialization in procedure declarations. In Fortran, this implies the `SAVE` attribute, causing the pointer to be shared across threads, leading to potential race conditions. - In NESDIS Emissivity modules: Remove explicit `SAVE` attributes for local coefficient arrays (`coe`). These arrays are assigned at runtime; sharing them across threads creates race conditions during assignment.
Inside the channel-thread !$OMP PARALLEL DO loops, every status-returning
call assigned its result to the function-result variable Error_Status,
which is SHARED across threads. Concurrent threads writing it raced, and
worse, a thread that hit FAILURE then ran a later successful call (or a
sibling thread did) overwrote it with SUCCESS -- so the post-loop
"IF (Error_Status == FAILURE) RETURN" could be skipped and the routine
return SUCCESS with garbage output. (In CRTM_K_Matrix_Module the same
variable was also being written via the lowercase typo "Error_status =",
which Fortran treats as the same symbol.)
Replace the in-loop usage with a thread-private Err_Thread for each call
and a thread_error accumulator combined with REDUCTION(MAX:thread_error)
(SUCCESS < INFORMATION < WARNING < FAILURE, so MAX keeps the worst). The
post-loop check now keys off the reduced value:
IF ( thread_error == FAILURE ) THEN
Error_Status = FAILURE
RETURN
END IF
Also drop AAvar from the PRIVATE clauses: it is dimensioned
(n_channel_threads) and indexed AAvar(nt), so it should be SHARED like
its siblings RTV(nt)/AtmOptics(nt)/CSvar(nt)/... rather than forcing each
thread to allocate the whole array privately.
No change to normal-path results; full ctest set unchanged before/after.
…28359869325e1a0505f85
`Zeeman/Zeeman_Utility.f90` (the geomagnetic-field LUT loader: load_bfield_lut / compute_bfield / compute_kb_angles, with a module-level `INTEGER, SAVE :: BField`) is not USEd by anything in the runtime library -- the runtime ODZeeman path uses BeCoeff (via CRTM_BeCoeff). Its only consumer is the standalone offline tool src/Coefficients/BeCoeff/BeCoeff_ASC2NC, which builds it via its own Makefile. Removing it from src/CMakeLists.txt drops one piece of unused module-level SAVE state from libcrtm's static footprint (thread-safety audit, #111). The source file is kept for the offline tool. ctest 195/195 unchanged.
Documents the threading model and the host-application thread-safety contract (#111 objective 4): - OPENMP=ON/OFF build option (added to the CMake-variables list too); - OMP_NUM_THREADS controls the thread count; unset/empty -> 1 thread (coerced in CRTM_Init); - which entry points parallelize over what (Forward/TL/K: channels +/- profiles; Adjoint: profiles); - CRTM_Init / CRTM_Destroy are single-threaded; the RT entry points are read-only after Init but must be called from a single host thread; non-overlapping array arguments; - the legacy-classic-ifort serial fallback for the K-matrix channel path.
New test `test_OMP_Consistency` (omp_tests group, sensors atms_n21, abi_g18, cris399_npp): runs CRTM_Forward and CRTM_K_Matrix on identical input at OMP_NUM_THREADS = 1 (serial reference) and then at a sweep of thread counts (2, 4, 8, ... up to OMP_GET_MAX_THREADS()), asserting every RTSolution / RTSolution_K / Atmosphere_K / Surface_K is bit-identical to the serial run (CRTM's `==` operators, i.e. exact equality). This is a self-consistency test -- no reference data files -- so it directly targets the channel-thread race classes fixed for #111: unindexed RTV broadcasts, off-by-one / overshoot in the channel-chunk math (OOB writes that SIGSEGV'd at OMP>=4), per-channel NLTE/Zeeman predictor contamination (wrong Atmosphere_K at OMP>=8), and shared Error_Status writes. No-op (PASS) under a non-OpenMP build or a single-thread host. ctest 198/198 (195 + 3).
…fix correctly) The `dimensions` dummy argument of FitCoeff_1D/2D/3D_Create was explicit-shape `dimensions(1)`/`(2)`/`(3)`, which forces the compiler to materialize an array temporary when a caller passes a non-contiguous actual (ifort `-check arg_temp_created` warning observed via MWwaterCoeff_FASTEM6 -> FitCoeff_SetValue, reported in #192). Changed to assumed-shape `dimensions(:)`; the bodies already index `dimensions(1..N)` explicitly, the routines disambiguate inside the FitCoeff_Create generic via `self`'s type, so this is semantics-neutral. Note: an earlier attempt at this (12e5be0) mistakenly edited the *type components* (`INTEGER(Long) :: Dimensions(1) = 0` -> `Dimensions(:) = 0`, which is invalid Fortran for a non-allocatable component) and was correctly reverted (56db609). This applies the fix Andrew actually proposed -- the *_Create dummy args. ctest 198/198 (RELEASE and DEBUG builds both clean).
…rt Tbs After the input-type SELECT CASE, NESDIS_ATMS_SNOWEM unconditionally did `ANY(Tbs((/1,2,3,4,5/)) ...)` and then `CALL ATMS_SNOW_ByTBTs_D(..., Tbs, ...)`, which (a) references `Tbs` even though it is an OPTIONAL argument and (b) indexes elements 1..5 regardless of `SIZE(Tbs)`. When the snow-surface SfcOptics call comes from a sensor/SensorData configuration with fewer than five window-channel TBs (or none), this is the out-of-bounds crash in ATMS_SNOW_ByTBTs_D reported in #192 (seen with `-check bounds` and as a release-mode segfault). Now the diagnosis-based path is only taken when `PRESENT(Tbs) .AND. SIZE(Tbs) >= nwch`; bad/non-finite values (including NaN, via x/=x) fall back to ATMS_SNOW_ByTypes; and when Tbs is absent/too short the result from the SELECT CASE above is kept. No behaviour change for the well-formed >=5-TB case. ctest 198/198 (RELEASE & DEBUG builds clean).
…rity with #192) NESDIS_ATMS_SeaICE unconditionally fed its window-channel TBs into ATMS_SeaICE_ByTbTs_D with no validity check of its own (it relies entirely on the SfcOptics caller's `< 50 / > 500` gate, which doesn't catch NaN). It's structurally safer than the ATMS_SnowEM routine fixed in de331d9 -- `Tbs` is a mandatory explicit-shape(5) dummy and there's a single guarded caller, so no out-of-bounds risk -- but for parity, only call ATMS_SeaICE_ByTbTs_D when ALL(tbs >= 50 .AND. tbs <= 500) (the >= test also rejects NaN); otherwise keep the default em_vector (0.82/0.85). Behaviour unchanged for well-formed inputs. ctest 198/198.
The two test_ChannelSubset regression tests never exercised the channel-thread path: they don't set OMP_NUM_THREADS and CRTM_Init coerces an unset/empty value to a single thread, so a normal `ctest` run checked subsetting only serially. The subset + OpenMP-over-channels bug from #164 is fixed (verified manually across OMP_NUM_THREADS 1..16), but nothing automated covered it. Add test/mains/regression/forward/test_ChannelSubset_OMP: same self-consistency pattern as test_OMP_Consistency (no reference files; sweeps OMP_NUM_THREADS {1,2,4,...,max} asserting Forward + K_Matrix are bit-identical to serial), but first applies three subset shapes chosen to stress the channel-chunk / inactive- channel bookkeeping in CRTM_Forward / _K_Matrix: * "sparse" - positions spread across the range -> every chunk mixed * "front" - first 7 positions -> trailing chunks fully inactive * "split" - 4 front + 3 tail -> empty middle chunks (the historical off-by-one in the per-channel output index) Subsets are built from positions in CRTM_ChannelInfo_Channels(), not channel numbers, so it works for sparse SpcCoeffs (e.g. cris399_npp). It also asserts CRTM_ChannelInfo_n_Channels and the active-channel list match the request. Registered in the omp_tests group with iasi_metop-b, cris399_npp, atms_n21. Also harden CRTM_ChannelInfo_Subset: after the matching sweep, verify the whole requested list was consumed (j == n+1). If not -- a requested channel is not in this sensor (the MINVAL/MAXVAL test only bounds the range, not membership) or the list has duplicates -- return FAILURE instead of silently leaving fewer channels active than asked for. ctest -R 'ChannelSubset|OMPoverChannels|OMP_Consistency' -> 9/9.
Introduce PARMIO_FREQ_THRESHOLD = 200.0_fp (GHz) and gate the Forward / TL / AD dispatchers in CRTM_MW_Water_SfcOptics.f90 on (PARMIO LUT loaded) .AND. (channel frequency >= threshold). Below the threshold, fall through to the legacy LowFrequency_MWSSEM (< 20 GHz) or FASTEM/Fastem1 path. Threshold value chosen from obs-space validation against an ATMS-NPP cycle on #303: PARMIO offers no clear-ocean RMSE improvement at 165-183 GHz versus FASTEM6, and degrades 88 GHz residuals in cold/high-wind regimes (foam-treatment driven). 200 GHz cleanly excludes the entire ATMS band while preserving PARMIO for the >= 200 GHz channels of AWS / MWHS / ATMS-NG class sensors, where FASTEM6 extrapolates non-physically.
The frequency gate from the previous commit routes every ATMS-NPP
channel (max formal frequency 183.31 GHz) through the legacy
FASTEM/Fastem1 path, so these tests can no longer observe a
PARMIO vs FASTEM difference and would fail their dispatch-routing
assertions:
- test_PARMIO_Lifecycle (ATMS PASS_WITH_LUT delta gate)
- test_PARMIO_Dispatcher (per-call flag dispatch on ATMS)
- test_PARMIO_ATMS_ClearOcean_ObsSpace (ATMS obs-space A/B)
- test_PARMIO_FASTEM_ATMS_GridSweep (ATMS SST x U10 x SSS grid;
was never tracked in git, deleted from working tree)
Their dispatcher-routing intent is now covered by the surviving
A/B tests (DeltaSweep on AWS, AWS1_ObsSmoke, GMI_ObsSpace) and
by the obs-space validation in #303.
DeltaSweep / AWS1_ObsSmoke / GMI_ObsSpace previously toggled
opt(:)%Use_PARMIO_Model between two CRTM_Forward calls on the
same inputs. With the dispatcher gated on frequency, that flag
no longer steers anything, so each test now does an explicit
two-phase comparison:
Phase 1: CRTM_Init() with no PARMIOCoeff_File -> LUT not loaded
-> simulate -> capture TBs into a per-(channel, case)
tb_fastem buffer
Phase 2: CRTM_PARMIOCoeff_Load(lut_file)
-> simulate -> capture into tb_parmio buffer
-> CRTM_PARMIOCoeff_Destroy()
Compare phases, write the existing residual/summary CSVs.
For DeltaSweep this adds a single Run_Grid_Sweep internal
subroutine; for the two CSV-driven obs-space tests it adds a
Load_All_Scenes helper that pre-reads scenes so the same
sequence can be replayed in each phase.
This preserves the existing CSV schemas and pass/fail gates
without depending on a runtime opt-in selector, and works
unchanged whether the dispatcher routes a given channel through
PARMIO (>= 200 GHz) or FASTEM (< 200 GHz).
With the dispatcher gated on frequency (>= 200 GHz) and the
ATMS-only regression tests gone, nothing reads
SfcOptics%Use_PARMIO_Model or Options%Use_PARMIO_Model anymore.
Strip the field plus all surrounding plumbing:
- CRTM_Options_Define.f90: type field, SetValue arg / decl /
assignment, docstring, equality, print, binary read/write.
- CRTM_SfcOptics_Define.f90: type field, print, two equality
sites.
- CRTM_Forward_Module.f90, CRTM_Tangent_Linear_Module.f90,
CRTM_Adjoint_Module.f90, CRTM_K_Matrix_Module.f90: all 12
SfcOptics(...)%Use_PARMIO_Model = Opt%Use_PARMIO_Model
propagation lines.
The remaining surviving regression tests use
CRTM_PARMIOCoeff_Load / CRTM_PARMIOCoeff_Destroy directly to
control LUT presence and never touched the flag, so they
continue to build clean.
Drop-in PARMIO support: when no PARMIOCoeff_File argument is supplied, CRTM_Init checks File_Path/PARMIO.MWwater.EmisCoeff.nc and loads it if present. Absence falls through silently to the FASTEM-only path, byte-identical to a pre-PARMIO build. The PARMIOCoeff_File argument is retained as an explicit override for non-standard locations; in that case a missing file remains a hard error so callers asking for a specific LUT get a clear failure rather than silent fallback. Operationally: drop PARMIO.MWwater.EmisCoeff.nc into the CRTM coefficient directory alongside FASTEM6.MWwater.EmisCoeff.nc and the dispatcher picks it up for >= 200 GHz channels. Remove the file (or ship CRTM without PARMIO support compiled) for legacy behavior. UFO, fv3-jedi, mpas-jedi need no PARMIO-specific yaml or code; this commit completes the move to a fully caller-agnostic PARMIO surface.
… into feature/btj_REL-3.2.0
BTJ uploaded the tarball and updated both md5 pins in 81632f4. Four documents still described it as rolled-but-unpublished and quoted the superseded checksum, and three still told users to build with -DFIX_FILE_PATH because a default build would fetch the wrong tree. All of that is now wrong in the user's favour, which is the worst kind of stale: it sends people down a workaround they no longer need. The published tarball is NOT the roll these documents described. It was re-rolled on 2026-08-04: superseded 3,377,500,279 bytes 7cd36fb18e3c69d5f4399a31009cc4ce published 3,377,514,134 bytes bc25af8f83e9ab7b5ed2080507aded15 Updated RELEASE_NOTES_v3.2.0.md, README.md, REL-3.2.0_changes_vs_develop.md and the GSI interface plan to the published values, and replaced the FIX_FILE_PATH workaround with a note that a default cmake now downloads and checksum-verifies the correct tree, keeping FIX_FILE_PATH documented as the option it is rather than the necessity it was. The changes doc keeps an explicit note recording the superseded checksum and saying to discard any copy carrying it, because that roll circulated in earlier revisions of these files. Verified rather than taken on trust: - Re-rolled tarball against the staging tree, file by file: 1440 files, zero differences. The re-roll changed content, not membership. - local md5 == published md5 == both pins == bc25af8f. - A default configure with no FIX_FILE_PATH succeeds, with cmake checksum-verifying the file instead of erroring. - That last check short-circuits the actual download, so the server copy was checked separately by byte-range: three 1 MB ranges (start, middle and end) fetched from bin.ssec.wisc.edu all md5-match the local file, alongside an exact Content-Length match. Short of pulling 3.4 GB this is the strongest available evidence that the published bytes are the bytes we verified.
The inner channel PARALLEL DO declared ln FIRSTPRIVATE and accumulated ln = ln + (start_ch - 1) - n_inactive_channels(nt) per iteration, which is correct only when every thread executes exactly one Thread_Loop iteration. NUM_THREADS is a request, not a guarantee: when the runtime delivers fewer threads (nested regions under a thread limit), a thread that takes a second chunk carries its previous chunk's final ln and indexes RTSolution past n_Channels. Fix: capture ln_base before the region, declare ln PRIVATE, and rebuild ln = ln_base + (start_ch - 1) - n_inactive_channels(nt) on every iteration. Applies to CRTM_Forward, CRTM_Tangent_Linear and CRTM_K_Matrix; CRTM_Adjoint has no channel threading. In the K module ln_base is assigned above the legacy-ifort gate (#231) so both preprocessor branches read a defined value. Reproduced pre-fix with nvfortran 25.5 -Mbounds: "Subscript out of range for array rtsolution, subscript=23, upper bound=22" (CRTM_Forward_Module.f90:996, atms_n21, OMP_NUM_THREADS=4). Post-fix the same build passes test_OMP_Consistency at 4 and 19 threads and under OMP_THREAD_LIMIT=3, plus test_ChannelSubset_OMP and test_Unit_MultiSensor_SingleCall. gfortran 13.3 -fcheck=bounds passes the full sweep for atms_n21, abi_g18, v.abi_g18 and cris399_npp, including OMP_THREAD_LIMIT 2/3/5. Note that gfortran cannot falsify this defect class (libgomp grants inner teams their full request even under OMP_THREAD_LIMIT), so nvfortran is the demonstrating compiler.
After each sensor's channel region, ln advanced by ln = ln + n_sensor_channels - n_inactive_channels(n_channel_threads+1), which assumes Thread_Loop left the outer ln untouched. That holds only when the OMP directive is compiled in (PRIVATE ln). On the serial paths, an OPENMP=OFF build on any compiler, or the legacy-ifort K-matrix gate (#231), the loop body mutates the outer ln to ln_base + n_active, and the post-loop advance then adds n_active again, so sensor 2 and later of a multi-sensor single call index past RTSolution. In a RELEASE build without bounds checking that is silent out-of-bounds writes. Confirmed with an OPENMP=OFF gfortran -fcheck=bounds build: test_Unit_MultiSensor_SingleCall died with "Index '31' of dimension 1 of array 'rtsolution' above upper bound of 20" (amsua_metop-a's 15 channels advanced twice, so mhs_n19 started at row 31 of 20). The failure is identical with and without the previous commit; it predates it. This defect class predates 3.2.0 as well: 3.1.4's plain per-sensor assignment made sensor 2 silently overwrite sensor 1 on the same paths. Rebuild the advance from invariant inputs instead: ln = ln_base + n_sensor_channels - n_inactive_channels(n_channel_threads+1) in CRTM_Forward, CRTM_Tangent_Linear and CRTM_K_Matrix. On the OMP path this is bit-identical to the old line (the outer ln still equals ln_base when the region ends); on the serial paths it replaces the double-count with correct indexing. CRTM_Adjoint has no channel chunking and needs no change. Verified: the OPENMP=OFF bounds build now passes test_Unit_MultiSensor_SingleCall 20/20; gfortran and nvfortran bounds builds pass the OMP consistency sweeps (to 19 threads, thread limits 2/3/5), channel-subset and multi-sensor tests; gfortran RELEASE full suite 238/238.
nvfortran 25.5 rejects the remaining generic-procedure form with NVFORTRAN-S-0155 (could not resolve generic procedure unittest_assert), which stops the nvfortran build in the test tree. The type-bound form ioTest%Assert / ioTest%Passed matches the rest of the file and compiles on gfortran, ifx and nvfortran.
…omething test_forward_OMP_Speedup_cris-fsr_n21 flaked under nvfortran, failing a full suite run at 1.111x against the 1.20x threshold and passing the next. RUN_SERIAL was already set, so it was not contention from sibling tests. The cause is that a single wall-clock sample is not a stable measurement. Four consecutive standalone runs of the identical binary on an idle host gave 2.244x, 1.787x, 2.247x and 1.627x, a spread of roughly 100 percent straddling the threshold. A red result therefore carried no information: a real threading regression and a badly landed sample looked the same. Each phase is now timed N_TRIALS=3 times and the fastest trial kept, for the serial and the parallel phase alike. The fastest run is the one least perturbed by scheduler, thermal and page-cache effects, so this suppresses transient dips without inflating the result. Measured after the change, six nvfortran runs: 1.569x to 1.772x, a 13 percent spread, with the worst case 31 percent clear of the threshold. gfortran 3.059x and ifx 3.630x. The test costs about 45 s on nvfortran and about 22 s on the other two, up from about 16 s. Verified: gfortran, ifx and nvfortran.
Four tests are roughly 75 percent of suite CPU under gfortran and ifx and about 84 percent under nvfortran, while each of the other 234 runs in under four seconds: test_UV_NO2_TLAD, test_VectorRT_TLADK, test_TEMPO_UVVIS_Physics and test_OMPS_UV_Physics. Deferring them cuts a routine ctest run by about 4.5x (measured back to back on one host: 380.0 s with them, 84.4 s without). They are gated by a new BUILD_TIER2_TESTS option, default OFF, and carry the ctest label "tier2" so they can also be selected with `ctest -L tier2` or excluded with `ctest -LE tier2` once registered. This follows the label convention already used in ufo, fv3-jedi and mpas-jedi, with a build-time gate added on top so the default suite is fast without requiring every caller to remember an exclusion flag. Only add_test is gated, never add_executable. The four executables are still built on every build, so compiler coverage of those sources is preserved: an nvfortran compile defect was found in a test source earlier in this release cycle, and a design that gated the build would have hidden it. A consequence of gating registration alone is that toggling the option costs a reconfigure and no recompilation. test_OMP_Speedup deliberately stays in the default tier. It is the only test that measures whether the threading design pays off, and at about 22 s it is affordable. The README section records when to run tier2: before tagging a release, and on any change to the RT or OpenMP threading path. All four use N_PROFILES=2 and do not pin OMP_NUM_THREADS, so they engage the nested channel-threading path, and test_OMPS_UV_Physics (4 sensors) and test_TEMPO_UVVIS_Physics (2 sensors) are among the strongest coverage of multi-sensor RTSolution indexing under it. That is exactly where the out-of-bounds defect fixed in 13cd613 lived. Verified on gfortran, ifx and nvfortran, identical on all three: executables 4/4 built, OFF registers 234 tests with 0 labelled tier2, ON registers 238 with 4 labelled, and 0 compile or link actions on toggle.
…ent loss Two coefficient findings from the 2026-08-01/04 audit work, documented against the staged fix tree. The ABI assumption audit (JCSDA/CRTMv3 issue #347) confirmed the pending- regeneration flag on abi_g19 and found worse: the staged 2024 STAR file was a self-described test article whose fitted-CO2 extrapolation produced a measured -1.67 K ch16 O-B bias against 1279 GOES-19 clear-ocean superobs, plus spurious stratospheric water Jacobians in the window channels. Causal proof by retraining. The entire ABI ODPS family was regenerated at the 2026.5 gas epoch with per-flight-model CWG SRFs authenticated against NOAA NCC, and staged over the old files. The v.abi_g19 VIS half was not touched and still carries the stale CO2 range. Separately, VIS/UV products can lose an ODPS component because component_significance is expressed in dimensionless surface-transmittance RMSE rather than the Kelvin it was documented as, while the acceptance gate sits in the same units at 1.0e-3. Any VIS/UV product built at that threshold could drop a component worth a channel's entire error budget. Refits at 1e-4 are tabulated. NOTE, and this blocks tagging: the published tarball (bc25af8f83e9ab7b5ed2080507aded15, rolled 2026-08-04 11:45) predates the ABI family regeneration staged at 18:52 the same day. Ten files differ, verified by extracting from the tarball and comparing md5 against the staged tree: SpcCoeff/netCDF/{abi-81K_g17,abi_g16,abi_g17,abi_g18,abi_gr}.SpcCoeff.nc TauCoeff/ODPS/netCDF/{abi-81K_g17,abi_g16,abi_g17,abi_g18,abi_gr}.TauCoeff.nc abi_g19 was staged before the roll and is present in the tarball, so the shipped ABI family is currently internally inconsistent: one platform regenerated, five not. The tarball must be re-rolled and its md5 updated in Get_CRTM_Binary_Files.sh, the CMake download hash, and the release documents before REL-3.2.0 is tagged.
The published tarball predated the ABI ODPS family regeneration. It was rolled at 11:45 on 2026-08-04; the regenerated abi_gr, abi_g18, abi_g17, abi_g16 and abi-81K_g17 SpcCoeff and TauCoeff files were staged at 18:52, seven hours later, so ten files never made it into the archive that went to the FTP site. Caught by md5 of files extracted from the published tarball against the staging tree rather than by timestamps: abi_g18.SpcCoeff.nc read ce9a40a6d2e5 in the tarball against 3c3f2028cb2e staged. abi_g19 was staged on 08-03 and did make the roll, so the published ABI family was internally inconsistent, one platform regenerated at the 2026.5 gas epoch and five still on the old basis. For a family users routinely intercompare across platforms that is worse than shipping all-old or all-new. Re-rolled at 22:52. Verified before the previous archive was replaced: 1440 files matching the staging tree exactly, same top-level layout, and every extracted ABI file md5-identical to its staged counterpart. size 3,377,517,263 bytes (was 3,377,514,134) md5 2170582827633c83946e6b4b97ee7c7d (was bc25af8f83e9ab7b5ed2080507aded15) Both functional pins updated and checked against the artifact on disk. The release documents record the new value, and the older figures are kept as dated history rather than deleted. The upload has NOT happened. Until it does, the pins deliberately disagree with the file bin.ssec.wisc.edu still serves, so a default build will fail its checksum. The pins describe the tree the release ships, not the stale one. After uploading, confirm the served Content-Length is 3377517263.
…I change
The coefficient section asserted the two efforts holding the roll had landed and
left the actual scope of the change undescribed. It now carries the measured
classification of every file against the v3.1.4 baseline (fix_REL-3.1.2.0, md5
0e5888cae80aa674b2e67ecd4490317d, the tarball v3.1.4 itself pinned):
identical 401, metadata-only 779, data-changed 26, new 234, retired 3605
Shared plus new reconciles to the 1440 files the tarball actually contains. The
table is transcribed from census.json and was checked against it programmatically
rather than by eye.
Three things the numbers needed saying out loud:
- Only 26 files can move a brightness temperature for an existing sensor. The
779 metadata-only files are the provenance backfill and change nothing
radiometrically.
- "3605 retired" overstates the loss: 2558 are format drops from the
Big_Endian/Little_Endian split disappearing, and of the genuine netCDF
retirements 14 are renames with verified counterparts (atms_j2 -> atms_n21
and siblings, JPSS-2 became NOAA-21 at launch). A user whose sensor appears
to have vanished should check the rename list before filing a bug.
- The ABI infrared family was regenerated late in the cycle and all six
products carry new numbers. ABI users must not carry forward bias
corrections trained on the old coefficients.
The census that backed the earlier text was refreshed on 2026-08-01, before the
ABI regeneration, and classified abi_g16 and abi_g17 SpcCoeff as identical to
v3.1.4 with the rest metadata-only. Left alone it would have shipped a release
note asserting the ABI infrared coefficients were unchanged, in the same release
that rebuilt them to remove a measured -1.67 K ch16 bias. Re-running the census
against the restored baseline moved exactly twelve files into data-changed, with
identical -2 and metadata-only -10 accounting for it and nothing else in the
tree moving.
Also corrects the "a default build now works" claim: until the re-rolled archive
is uploaded, the pins deliberately disagree with the served file and a default
build will fail its checksum.
… server Verified against bin.ssec.wisc.edu on 2026-08-05: Content-Length 3377517263, Last-Modified Wed, 05 Aug 2026 03:15:19 GMT. The size alone distinguishes the re-roll from the superseded archive (3,377,514,134), so the served file is definitively the one carrying the ABI regeneration. Flips the three documents that were written for the pending state: the release notes status line and download table, the tarball note in the changes document, and the GSI staging section.
…e dispatch CRTM_VISsnowCoeff_Load parsed the scheme from the filename prefix but returned SUCCESS after printing FAILURE for an unrecognised prefix, in both the CASE DEFAULT arm and the no-dot filename branch. A typo'd or renamed VISsnowCoeff_File therefore let CRTM_Init succeed with no visible snow table loaded, and Compute_VIS_Snow_SfcOptics then returned SUCCESS having computed nothing, leaving stale SfcOptics contents in the RT. Both loader arms now return FAILURE, and the dispatch reports FAILURE if reached with neither table loaded (unreachable in normal runs, since CRTM_Init loads a VISsnow table unconditionally for any VIS or UV sensor list). Also corrects the CRTM_Init doc header: IGBP.VISsnow and USGS.VISsnow do not exist and their prefixes are rejected; the valid files are NPOESS (default) and SNICAR, selected by the text before the first dot.
…cal range The 4-point Lagrange interpolation across SEcategory reflectance spectra overshoots [0,1] where the tabulated spectrum has sharp structure. The NPOESS VIS snow table holds exact zeros at 4000 and 5000 cm-1 (and again from 6060 to 6451 cm-1) with positive neighbours, giving a reflectance near -0.03 at 2.251 um for both snow types. The visible-path direct-reflectivity limiter clamped only above one, so the negative reflectance scaled straight into a negative top-of-atmosphere radiance (-0.17 mW/(m2 sr cm-1) for VIIRS M11 over old snow; the factor 5.91 between reflectance and radiance matches solar irradiance times cos(45)/pi times two-way transmittance) and then a NaN brightness temperature through LOG of a negative argument in CRTM_Planck_Temperature, which is also what raised IEEE_INVALID at exit of any affected run. The defect is inherited, not a 3.2.0 regression: v3.1.4 reproduces -0.167 end to end on its own library and its own binary NPOESS table (v.viirs-m_j2, CloudCoeff loaded to avoid its VIS hang). Clamp the interpolant into [0,1] at the source in SEcategory_Emissivity and add the symmetric below-zero arm to the visible limiter in CRTM_RTSolution. Results change only where the clamp engages; no existing baseline moved on gfortran, ifx, or nvfortran (235/235 each).
… LUT Before this test the SNICAR table's only coverage was an I/O check that the file parses. The test computes radiances through the full RT for v.viirs-m_n21 over a fully snow-covered scene and asserts: the table is actually consumed (the NPOESS control differs), physical bounds, monotonic SWIR decrease with coarsening grain (52x at 1.613 um from 50 to 1500 um), depth and density response with an invariant NPOESS control, non-negative radiance and NaN-free brightness temperature on the default NPOESS path, finite out-of-LUT behaviour, and hard rejection of an unrecognised classification prefix. The solar-angle check asserts illumination geometry only: the table's angle dimension is labelled Solar Zenith Angle but is interpolated at the RT view/quadrature angles, so the solar zenith never reaches the table; that discrepancy is with the table's author and the assertion must not be strengthened until it is settled. The CMake registration gates on the source files in the fix tree and stages its own testinput links, like the TEMPO and OMPS blocks. Gating on the build-dir testinput does not work: bulk staging runs later in the file, so on a fresh configure the destination check is always false and the test silently drops out of the suite while still building and running by hand.
…ed libcrtm CMake links with RUNPATH by default, and LD_LIBRARY_PATH takes precedence over RUNPATH at load time. The oneapi spack environment carries its own crtm-3.1.2 package on LD_LIBRARY_PATH, so every ifx test executed spack's CRTM 3.1.2 library against this build's 3.2.0 module interfaces: absent optional arguments arrived in CRTM_Init as present garbage, format resolution fell back to Binary with an empty effective path, and 230 of 235 tests failed while printing the loaded library's own CRTM Version v3.1.2 banner. Classic RPATH outranks LD_LIBRARY_PATH; with --disable-new-dtags the same suite passes 235/235 with the spack path still present. Known residual: nvfortran still emits RUNPATH (its driver appears to re-enable new dtags after user flags). Its environment carries no crtm package today, so there is no exposure there, but the guard does not cover it.
…anges recorded The SNICAR bullet now shows the real selection mechanism (VISsnowCoeff_File by filename prefix; there is no VISsnowCoeff_Scheme argument), states that a radiance-level physics test now exists, and lists the two remaining limitations, both deferred beyond v3.2.0: the table's angle dimension is labelled solar zenith but is interpolated at the RT view angles, and the forward path applies no LUT bounds guard while TL and AD return zero out of bounds. The angle question is pending confirmation with the table's author. Behavior-change entries 18 (reflectance clamp) and 19 (loader hard-fail) added, with a Known-issues cross-reference for the deferred SNICAR items. CLAUDE.md's overview version corrected from v3.1.2 to v3.2.0; the stale label camouflaged the wrong-library version banner during the ifx investigation.
CRTM_Forward, CRTM_Tangent_Linear and CRTM_K_Matrix each raise max-active-levels to enable their nested channel loop, but that setting is global to the OpenMP runtime and was never put back. A host that does its own threading therefore found its nesting policy silently replaced by a CRTM compute call, which can turn the host's own nested regions from serialised into thread-spawning and oversubscribe the machine. CRTM_Adjoint never sets the level at all, so it inherited whatever the previous forward or K-matrix call happened to leave behind. Each routine now records the caller's value before changing it and restores it on every exit path. No computational code is touched, so no baseline can move. Verified by A/B against the pre-fix library with the same test program: from a caller policy of 1, the old build reports 2 after both CRTM_Forward and CRTM_K_Matrix, the new build reports 1. Suite is 235/235 on gfortran 13.3 and ifx 2025.3.3, and the OPENMP=OFF build still compiles (the saved value is declared only under _OPENMP, so every restore is guarded).
Channel-level threading gives each thread its own AtmOptics, SfcOptics, RTV and scatter scratch structures, allocated per profile per call. That cost is set by the layer count and the stream and angle maxima, not by how many channels the thread then processes, so a thread holding a handful of channels pays far more to exist than it saves. The split only reached this path when threads outnumbered profiles, which is exactly the single-profile case GSI uses (crtm_interface.f90 passes dimension(1)). Measured there, threading was slower than not threading at all: one ATMS profile on 16 threads ran at 0.03x, that is roughly 30 times slower than the same build on one thread. JEDI and UFO were never exposed, since they pass the whole observation batch as profiles. n_channel_threads is now capped so each channel-thread owns at least MIN_CHANNELS_PER_CHANNEL_THREAD channels, with the measurements behind the value recorded alongside it in CRTM_Parameters. The expression only takes a MIN, so it can lower the thread count but never raise it, and it cannot create nesting where there was none. Where profiles already absorb every thread it is a no-op, because n_channel_threads is 1 before it runs. Measured effect (forward, wall clock, against the same build on one thread): one ATMS profile on 16 threads 0.03x to 0.86x, on 8 threads 0.10x to 0.96x; two ATMS profiles on 8 threads 0.33x to 1.21x; one CrIS-399 profile on 16 threads 0.32x to 1.07x. CrIS-FSR and every profiles-greater-than-threads case are unchanged. Suite is 235/235 on gfortran 13.3 and ifx 2025.3.3. This also replaces the disabled sensor-type test in the tangent-linear module, which was reaching for the same effect. Channel count is the better discriminator: the loss is driven by how little work a thread receives, which hits small infrared sensors too, while large infrared sounders are exactly where channel threading pays.
Each of the three routines discovered its thread count by opening a PARALLEL region, reading OMP_GET_NUM_THREADS inside a SINGLE, and closing it again. That is a full team spawn and join on every call, paid purely to read a number. The obvious replacement is not safe. OMP_GET_MAX_THREADS is equivalent only when called from serial code: inside a host's parallel region with nesting disallowed, a nested PARALLEL yields a team of one while OMP_GET_MAX_THREADS still reports nthreads-var. Measured directly, that is 1 against 8. Believing 8 there would size per-thread scratch for 8 channel threads and chunk the channels 8 ways, then run them serially. The cheap query is therefore used only where it is provably equivalent, that is when not already inside a parallel region and with dynamic adjustment off, since dynamic adjustment can also hand back fewer threads than nthreads-var. Otherwise the original spawn-and-count is kept, so behaviour in the nested case is unchanged. This helped every configuration, not only the ones the previous commit gated, because every call was paying the spawn. Measured (forward, wall clock, against the same build on one thread): one ATMS profile on 16 threads 0.86x to 1.00x, on 8 threads 0.96x to 1.00x; two ATMS profiles on 8 threads 1.21x to 1.86x; one CrIS-399 profile on 8 threads 1.45x to 2.12x; one CrIS-FSR profile on 8 threads 2.28x to 2.79x. Suite is 235/235 on gfortran 13.3 and ifx 2025.3.3. Taken with the previous commit, the single-profile case no longer loses anywhere: one ATMS profile on 16 threads went from 0.03x to 1.00x.
Adds test_Unit_OMP_Thread_Policy, covering the two defects fixed in the
preceding commits on the call shape GSI actually issues, one profile:
A. CRTM must restore the caller's max-active-levels. Exact, no timing.
B. Threading must not be dramatically slower than not threading. Bounded at
3x against a regression that measured about 30x, so a busy machine cannot
flake it while the defect cannot hide. RUN_SERIAL, since it times the
model.
Both checks were confirmed to fail against the pre-fix library and pass against
the fixed one: 22-channel sensor on 8 threads reports max-active-levels 2 after
both CRTM_Forward and CRTM_K_Matrix and an 11.5x slowdown before, against a
preserved policy and 1.03x after. A regression test that does not fail on the
broken code is worth nothing, so that check is part of the record rather than
an assumption.
Suite is 236/236 on gfortran 13.3 and ifx 2025.3.3.
Release notes gain behavior-change entries 20 (nesting policy restored to the caller) and 21 (channels no longer split below break-even), both stating that the defects are inherited from v3.1.4 rather than 3.2.0 regressions, who was exposed, and the measured effect. README's "OpenMP and thread safety" section is rewritten to describe the split as profiles first and channels only with threads left over, to say plainly that batching profiles is the single most effective thing a host can do, to explain why channel threading is gated on work per thread, and to record the new guarantee that CRTM leaves the caller's OpenMP settings as it found them. The GSI interface plan carried the claim that CRTM-internal channel OpenMP was "inert under GSI's per-profile call pattern". That was wrong and backwards: one profile with threads to spare was the worst case, not an inert one, because CRTM only reached the channel path when threads outnumbered profiles. The item now carries the correction, the measurement, and what still follows for GSI on older releases. Also catalogued in REL-3.2.0_changes_vs_develop.md alongside the earlier thread-safety work, kept separate from it because these are defects in how CRTM decides to use threads rather than races in how it uses them. NOTE: this commit also carries a pre-existing uncommitted change to REL-3.2.0_changes_vs_develop.md that is NOT part of the OpenMP work: the "2026-08-05 addendum" recording the six ABI TauCoeff files refreshed to Version 3 for the OPTRAN effective-target fix and the cris-fsr_n21 NLTECoeff replacement. It was swept in by staging the whole file and is called out here rather than rewritten back out, because the coefficient session that authored it is still running. Its paired file, REL-3.2.0_coefficient_inventory.md, remains uncommitted.
The suite passes 236/236 on nvfortran 25.5 as well as gfortran 13.3 and ifx 2025.3.3. Worth stating explicitly rather than leaving at two compilers: nvfortran is the one whose OpenMP runtime has previously surfaced threading defects that gfortran ran through silently, so it is the meaningful witness for this particular set of changes.
Note the Version 3 OPTRAN effective-target refresh on the six ABI-family TauCoeff entries (abi_g16/g17/g18/g19, abi_gr, abi-81K_g17) and the Version 2 NLTECoeff replacement for cris-fsr_n21. Prior files are archived under the test-data-release backup directories noted in each entry.
The final campaign roll of fix_REL-3.2.0.0.tgz is uploaded and live: size 3,377,422,223 bytes, md5 88995873986cf2b077808a75d1c56f83. Get_CRTM_Binary_Files.sh and test/CMakeLists.txt carry the new md5. RELEASE_NOTES_v3.2.0.md, REL-3.2.0_changes_vs_develop.md and the GSI interface plan record the roll and the confirmed upload. Verified before committing: the local tarball md5 matches the pin; the server copy matches the local roll in size (Content-Length 3377422223) and in a byte-compare of the first and last MiB; membership is 1440 files; the swapped ABI TauCoeff and CrIS NLTECoeff members extracted from the tarball are byte-identical to the staging tree.
Remove seven historical documents whose content is preserved in issues, committed docs, or git history: - netcdf_baselines_handoff.md (complete; facts in REL-3.2.0_changes_vs_develop.md section 9) - issue-281-comment.md, surface_jacobians_281.md (issue #281, closed, carries the record) - odps_group_modernization.md + _log.md (issue #342, closed) - downwelling_radiance_plan.md, telsem2_landem_jacobians_plan.md (design-record pointers posted on epic #357 with permalinks) Kept: the GSI and JEDI/UFO integration plans (external-consumer guidance), polarimetric conventions/roadmap (current capability + in-flight 3.3.x), parmio_permittivity_switch.md (decided, unimplemented: pending-work record). Also correct the coefficient inventory's reference to the superseded 2026-06-05 tarball.
Contributor
Author
|
Per @eap guidance on large PRs, before I move this PR out of draft, I will manually review each line of code and will post any self-review comments here. 95% of this has been fully adjudicated through various means, but regardless I will perform a thorough self-review. I'm sure it will reveal something. Most
Some markdown files are currently present as internal documentation, this serves as a guide for non-github users to navigate some of the most foundational changes to the code. These probably won't stick around long. I'll continue the self-review throughout the day(s) in the comments in this thread. |
|
Hi @BenjaminTJohnson. |
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
4 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.
CRTM v3.2.0 release integration
This PR brings the complete v3.2.0 release content from the
integration branch into the release branch. The head is a strict
superset of
develop(269 ahead, 0 behind as of 2026-08-20,including the #354/#356 cmake configure-time improvements pulled
in at jedi-bundle's request), and the base sits at the develop
tip, so the diff is exactly the release delta and merges
fast-forward clean.
Full details:
RELEASE_NOTES_v3.2.0.md(in this diff). Headlines:Library
netCDF (
.nc4to.nc; ODSSU converter included)GROUP_MW_O3, group index 7), TL/AD/Kparity verified
GROUP_UV_NO2) plusOPTRAN-for-VIS v2
the flag bit; the generator side now raises on zero-solar files
REL-3.2.0_changes_vs_develop.mdCoefficients (tarball
fix_REL-3.2.0.0.tgz, current roll2026-08-06, md5
88995873986cf2b077808a75d1c56f83, pinned inGet_CRTM_Binary_Files.shandtest/CMakeLists.txt).Note: the tarball remains in flux until the end of the
evaluation period; any re-roll updates the pinned md5 in the
same commit, so the pins are authoritative at merge time.
authenticated per-flight-model SRFs); users should expect BT
changes and retrain bias corrections
with changed numbers; 779 metadata-only provenance backfills
files, zero differences); the full delta classification against
v3.1.4 lives in
test-data-release/coeff_delta_REL-3.2.0/Verification state
(rc_build 2026-08-06)
REL-3.2.0_coefficient_inventory.mdDrafted for release review; merge fast-forwards the release
branch to the integration tip.