Atmosphere/surface split: surface sweeps, exact Lambertian albedo closure, scenario batching#230
Open
cfranken wants to merge 8 commits into
Open
Atmosphere/surface split: surface sweeps, exact Lambertian albedo closure, scenario batching#230cfranken wants to merge 8 commits into
cfranken wants to merge 8 commits into
Conversation
…urface Port the gchp-io prototype onto main: three opt-in rt_run hooks (atm_snapshot_callback fired per Fourier moment before the surface step, stop_after_atmosphere, m_max_override), an AtmosphereRTCache holding the pre-surface composite snapshot per moment, and a surface-phase replay (rt_run_surface / rt_run_multi_surface) that amortizes the Nz-layer accumulation across many BRDFs. Scope: noRS, single band, Lambertian/RPV/RossLi/CoxMunk (Canopy and Raman rejected with ArgumentError). Replay is bit-exact vs monolithic rt_run (61 new tests incl. Cox-Munk TMS replay and m_max widening). Design doc: proposals/surface_split_albedo_sweep.md.
LambertianClosure reads S̄ (spherical albedo from below), E_dw (BOA downward flux) and t_up = T⁻⁻u off the cached m=0 atmosphere blocks and evaluates the exact rank-one (Sherman-Morrison) closure R(a) = R_black + w₀·2a·E_dw/(1−a·S̄)·t_up — O(1) per albedo, with exact albedo Jacobian and closed-form inversion (invert_albedo). Agrees with the rt_run_surface replay to ~1e-15 rel. Guards: solar-only (per-source slots and SurfaceSIF rejected — SIF bounces albedo-dependently and is not captured by the closure term). AtmosphereRTCache gains cache_mode (:auto/:full/:slim): slim keeps the four NquadN² blocks only at m=0 and just J₀± above (Lambertian surface layers are exactly zero at m>0, so the skipped interaction is the bit-exact identity), cutting cache memory ~(m_max+1)-fold for Lambertian-only sweeps; :auto resolves via component_m_max.
Port ScenarioSweep/SweepResult/run_sweep from the gchp-io prototype: SZA × view-pair × BRDF batching where each SZA's atmosphere is built once and amortized across BRDFs via rt_run_multi_surface. Instead of the prototype's full model_from_parameters rebuild per SZA, run_sweep uses the new remake_geometry(model, params; sza, vza, vaz), which rebuilds only ObsGeometry + QuadPoints and shares optics/atmosphere/ solver/surfaces by reference — bit-exact vs a full rebuild (tested), so SZA sweeps skip HITRAN and Mie entirely. Also stabilizes the pre-existing RPV test flakiness (nondeterministic Infs/NaNs out of the threaded batched kernels, moves between monolithic and replay paths run-to-run — pre-existing on main, documented in the test NOTE block) with a BLAS pin + bounded rebuild-retry around the test bundle; equality assertions unchanged.
…, scenario sweeps
The atm_snapshot_callback captured the host slice τ_sum_all[:, end] while the monolithic surface step consumes the device-resident τ_sum_end_dev; rt_run_surface then passed the cached host Vector into create_surface_layer!, which would broadcast it against device arrays on GPU models. Snapshot τ_sum_end_dev instead — bit-identical on CPU, and the replay now preserves rt_run's host/device contract (Codex review finding).
…closure/sweep New student-facing User Guide page (pages/batch_processing.md): cost anatomy of a run, a decision table mapping what-changed → which tool → what's reused (surfaces via rt_run_atmosphere/rt_run_surface, Lambertian albedos via lambertian_closure, aerosol loading/microphysics and trace-gas/profile updates via BatchContext, geometry via remake_geometry/ run_sweep), a combined scenario-production-loop recipe with the update-scene → build-cache → sweep-surfaces ordering rule, and the full-rebuild list. Adds @docs entries for all 12 new exports (checkdocs = :exports would fail the docs build without them) and cache-invalidation warning blocks in the rt_run_atmosphere / BatchContext / lambertian_closure docstrings.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an atmosphere/surface split for the forward RT solve so many BRDFs/albedos/geometries can be evaluated against a fixed atmosphere without re-running the expensive adding–doubling layer accumulation. It adds an analytic Lambertian albedo closure (O(1) per albedo) on top of the cached atmosphere, and a scenario sweep driver that batches SZA × view-pairs × BRDF while reusing optics.
Changes:
- Add
rt_run_atmosphere/rt_run_surface/rt_run_multi_surfacewith anAtmosphereRTCache, implemented via newrt_runhooks (atm_snapshot_callback,stop_after_atmosphere,m_max_override). - Add
LambertianClosure(lambertian_closure,albedo_jacobian,invert_albedo) plus the:slimcache mode for Lambertian-family replays. - Add
ScenarioSweep/run_sweepandremake_geometryto amortize geometry rebuild and batch scenarios; expand docs and add substantial test coverage.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_surface_split.jl | New regression tests for bit-exact atmosphere/surface split replay, cache sizing/guards, and multi-surface sweeps. |
| test/test_scenario_sweep.jl | New tests validating remake_geometry bit-exactness and run_sweep output against monolithic references. |
| test/test_lambertian_closure.jl | New tests for Lambertian closure accuracy, Jacobian, inversion, guards, and :slim cache behavior. |
| test/runtests.jl | Wires the new testsets into the main CI suite. |
| src/vSmartMOM.jl | Exports the new cache/sweep/closure public APIs. |
| src/CoreRT/types.jl | Adds the AtmosphereRTCache type + docstring and integrates it into CoreRT types. |
| src/CoreRT/tools/update_model.jl | Documents cache invalidation after scene updates (update_model! / aerosol updates). |
| src/CoreRT/tools/scenario_sweep.jl | Adds ScenarioSweep, SweepResult, and run_sweep batching driver. |
| src/CoreRT/tools/model_from_parameters.jl | Adds remake_geometry for cheap geometry-only RTModel rebuilds. |
| src/CoreRT/tools/lambertian_closure.jl | Implements the analytic Lambertian albedo closure + Jacobian + inversion helpers. |
| src/CoreRT/rt_run.jl | Adds optional atmosphere snapshot callback, stop-after-atmosphere flag, and m-max widening hook. |
| src/CoreRT/rt_run_split.jl | Implements cache build (rt_run_atmosphere) and surface replay (rt_run_surface) plus multi-surface helper. |
| src/CoreRT/CoreRT.jl | Includes new split/closure/sweep modules and exports their public symbols. |
| proposals/surface_split_albedo_sweep.md | Adds a detailed design/proposal writeup and derivations for the new functionality. |
| docs/src/pages/concepts/05_surfaces.md | Documents surface sweeps, Lambertian closure usage, and points to code anchors. |
| docs/src/pages/batch_processing.md | New user guide page: decision table + recommended production-loop ordering + examples. |
| docs/src/pages/api/core_rt.md | Adds API docs sections for split cache, closure, and scenario sweep exports. |
| docs/make.jl | Adds the new “Fast Re-runs & Batch Processing” page to the docs nav. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+45
to
+49
| struct ScenarioSweep{FT} | ||
| sza::Vector{FT} | ||
| view_pairs::Vector{Tuple{FT, FT}} | ||
| brdfs::Vector{<:AbstractSurfaceType} | ||
| end |
…av overhaul Fixes the docs CI failure on this branch: remake_geometry's docstring references rt_set_streams via @ref, which no @docs block exposed — Documenter terminated at cross-reference resolution. rt_set_streams (and the quadrature types) now have API entries. Structure cleanup per audit: - Delete the pre-rewrite orphaned docs tree (14 files, ~1460 lines: design.md, vSmartMOM/{Overview,Types,CoreRTTheory}, old Absorption/, Scattering/, Surfaces/, Inelastic/ clusters) — unreachable from nav, superseded by the concepts arc, but still built at live URLs. All exported-symbol @docs entries they held are relocated to the live api/ pages (checkdocs = :exports stays satisfied). - Author the two missing schema pages (IO/Schema/canopy.md, spec_bands.md) and fix their three '(forthcoming)' dead links. - Rewrite gpu.md from stub wishlist to real content. - Nav: Tutorials moved up to follow User Guide; Input Schema becomes a 10-page sub-list; 'Configure a Scene' now points at the step-by-step guide (IO API page relabeled and moved after it). - Onboarding: For:/Next: headers on conventions / ConfigurationGuide / IO Overview; Quick Start now links Conventions and the new batch guide; index.md gets a Start-Here line. - release_notes.md: Unreleased section for the surface split, Lambertian closure, and scenario sweeps. - Consistency: GEOS-Chem H1 spelling, Library Overview label, dev_notes pointer for contributors, scattering-block naming note in aerosols.md. Local docs build: clean (0 errors, 0 new warnings).
Member
Author
|
Added two docs commits: |
…rt_albedo shape guards - rt_run_surface's component_m_max ctx set stream_l_cap = user_l_cap, inconsistent with the documented 2·Nstreams−1 contract that rt_run_atmosphere uses. Not a live bug (surface traits read only user_l_cap today) but consistency insurance if a future trait consults stream_l_cap; fixed here and in the matching test ctx. - invert_albedo only checked R_meas's VZA axis; since the retrieval loop is @inbounds, a wrong spectral length was silent UB rather than a clean error. Validate all three axes up front (DimensionMismatch), with regression tests. Declined Copilot's ScenarioSweep.brdfs abstract-eltype comment: a heterogeneous BRDF list is the intended use (can't be concretely typed) and it is not in a hot path (each element triggers a full RT replay).
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.
What
Evaluate many surfaces / albedos / geometries against one atmosphere without re-running the adding–doubling:
rt_run_atmosphere(model)runs the Fourier/layer loop once and snapshots the pre-surface composite per moment into anAtmosphereRTCache;rt_run_surface(cache, brdf)/rt_run_multi_surface(model, brdfs)replay only the surface phase (~1/Nz of a full run per surface), bit-exact vs monolithicrt_run. Implemented via three opt-inrt_runkwargs (atm_snapshot_callback,stop_after_atmosphere,m_max_override) — the default path is untouched. Port of the gchp-io prototype, re-mirrored against current main.lambertian_closure(cache)reads S̄ (spherical albedo from below), E_dw (BOA downward flux) and t_up = T⁻⁻u off the cached m=0 blocks;R(a) = R_black + w₀·2a·E_dw/(1−a·S̄)·t_upis exact (matches replay to ~1e-15), O(1) per albedo, with exactalbedo_jacobianand closed-forminvert_albedoretrieval. Lambertian-only caches are stored slim (full matrices at m=0 only).ScenarioSweep/run_sweep(SZA × view-pair × BRDF) on top ofremake_geometry, which rebuilds onlyObsGeometry/QuadPointsand shares optics by reference (bit-exact vs full rebuild → SZA sweeps skip HITRAN/Mie).@docscoverage for all 12 new exports, cache-invalidation warnings in docstrings.Scope guards (all
ArgumentError): elasticnoRSonly, single band, noCanopySurface. Deferred items + design/derivation:proposals/surface_split_albedo_sweep.md.Heads-up (pre-existing, not fixed here)
While stabilizing tests we confirmed a nondeterministic
Infs/NaNsfailure in the CPU threaded batched kernels (batch_inv!/⊠) with an RPV surface — reproduces on unmodified main, moves between code paths run-to-run with identical inputs, BLAS-thread sensitive.test_surface_split.jlcarries a documented bounded retry around the RPV bundle; the underlying kernel issue deserves a dedicated look.Test plan
test_surface_split.jl(61 — bit-exact identity/swap/RPV/Cox-Munk-TMS/m_max-widening/guards),test_lambertian_closure.jl(58 — closure vs replay ~1e-15, spectral vectors, Jacobian vs FD, inversion round-trip, slim==full),test_scenario_sweep.jl(54 — remake_geometry bit-exactness, sweep vs monolithic refs, view_mode guards).🤖 Generated with Claude Code