Atmosphere/surface split: surface sweeps, exact Lambertian albedo closure, scenario batching by cfranken · Pull Request #230 · RemoteSensingTools/vSmartMOM.jl

cfranken · 2026-07-11T00:22:43Z

What

Evaluate many surfaces / albedos / geometries against one atmosphere without re-running the adding–doubling:

Atmosphere/surface split — rt_run_atmosphere(model) runs the Fourier/layer loop once and snapshots the pre-surface composite per moment into an AtmosphereRTCache; 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 monolithic rt_run. Implemented via three opt-in rt_run kwargs (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.
Exact Lambertian albedo closure — 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_up is exact (matches replay to ~1e-15), O(1) per albedo, with exact albedo_jacobian and closed-form invert_albedo retrieval. Lambertian-only caches are stored slim (full matrices at m=0 only).
Scenario batching — ScenarioSweep/run_sweep (SZA × view-pair × BRDF) on top of remake_geometry, which rebuilds only ObsGeometry/QuadPoints and shares optics by reference (bit-exact vs full rebuild → SZA sweeps skip HITRAN/Mie).
Docs — new User Guide page "Fast Re-runs & Batch Processing" (decision table + student-facing production-loop recipe incl. the update-scene → build-cache → sweep-surfaces ordering rule), concepts §5 section, @docs coverage for all 12 new exports, cache-invalidation warnings in docstrings.

Scope guards (all ArgumentError): elastic noRS only, single band, no CanopySurface. 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/NaNs failure 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.jl carries a documented bounded retry around the RPV bundle; the underlying kernel issue deserves a dedicated look.

Test plan

Full suite green (40 testsets, pipefail): 173 new tests across 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).
Codex-reviewed (whole branch + per-commit); its GPU device-residency finding is fixed in 5ca7f36.

🤖 Generated with Claude Code

…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.

Copilot

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_surface with an AtmosphereRTCache, implemented via new rt_run hooks (atm_snapshot_callback, stop_after_atmosphere, m_max_override).
Add LambertianClosure (lambertian_closure, albedo_jacobian, invert_albedo) plus the :slim cache mode for Lambertian-family replays.
Add ScenarioSweep/run_sweep and remake_geometry to 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.

+struct ScenarioSweep{FT}
+    sza::Vector{FT}
+    view_pairs::Vector{Tuple{FT, FT}}
+    brdfs::Vector{<:AbstractSurfaceType}
+end


@ref

…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).

cfranken · 2026-07-11T01:09:20Z

Added two docs commits: 09730474 (Fast Re-runs & Batch Processing guide + API coverage for the new exports) and 41af602d (docs-structure cleanup: removes the stale pre-rewrite orphan tree (~1,460 lines), authors the missing Schema/canopy + spec_bands pages, rewrites the gpu.md stub, nav overhaul, release-notes Unreleased entry — and fixes the docs CI failure: remake_geometry's docstring @ref rt_set_streams had no @docs target). Local docs build is clean; the re-triggered CI docs job should now pass.

@inbounds

…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).

Copilot

🟡 Human review recommended

It introduces new public APIs and modifies core RT execution flow (surface coupling and per-moment control hooks), requiring careful human verification of solver invariants and performance/compatibility implications.

Review details

Suppressed comments (1)

src/CoreRT/tools/scenario_sweep.jl:49

ScenarioSweep stores brdfs as Vector{<:AbstractSurfaceType}, which is an abstract field type. This undermines the intent stated in the constructor comment (preserving a concrete BRDF element type) and can introduce dynamic dispatch when iterating sweep.brdfs in the sweep hot path.

Consider parameterizing ScenarioSweep on the BRDF element type (e.g. ScenarioSweep{FT,ST} with brdfs::Vector{ST}) and updating the keyword constructor to capture ST (brdfs::AbstractVector{ST}) so homogeneous surface sweeps remain fully type-stable.

Files reviewed: 58/58 changed files
Comments generated: 0 new
Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Copilot

🟡 Not ready to approve

ScenarioSweep stores brdfs with a non-concrete field type (Vector{<:AbstractSurfaceType}), undermining the intended type-stable/no-dynamic-dispatch hot path for sweep execution.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

src/CoreRT/tools/scenario_sweep.jl:49

ScenarioSweep stores brdfs as Vector{<:AbstractSurfaceType} (a Vector{T} where T<:AbstractSurfaceType). That field type is not concrete, so downstream code (e.g. rt_run_multi_surface(model_sza, sweep.brdfs)) can’t be inferred as having a stable BRDF element type, defeating the stated goal of avoiding dynamic dispatch in the hot path (see the comment near the constructor return).

Consider parameterizing ScenarioSweep on the BRDF element type (e.g. ScenarioSweep{FT,B<:AbstractSurfaceType} with brdfs::Vector{B}) and updating the keyword constructor to preserve/infer B from the input brdfs vector.

Files reviewed: 59/60 changed files
Comments generated: 0 new
Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Copilot

🟡 Human review recommended

It introduces new public APIs and execution paths in the core RT loop plus broad IO/docs changes, and the remaining review findings touch type-stability and API contract enforcement in newly added batch/sweep infrastructure.

Review details

Suppressed comments (3)

src/CoreRT/tools/scenario_sweep.jl:49

ScenarioSweep stores brdfs as Vector{<:AbstractSurfaceType}, which is a non-concrete field type and undermines the stated goal of preserving a concrete BRDF container for type-stable hot-path dispatch. Consider parameterizing the struct by the concrete BRDF vector type so brdfs remains fully inferred when passed into rt_run_multi_surface.
src/CoreRT/tools/scenario_sweep.jl:109
With ScenarioSweep parameterized on the concrete brdfs container type, the constructor should avoid calling ScenarioSweep{FT}(...) (which can’t supply the second type parameter) and instead build brdfs_v once and let Julia infer the full ScenarioSweep{FT,B} type from the arguments.
src/CoreRT/tools/model_from_parameters.jl:685
remake_geometry relies on params being geometry-consistent with model (quadrature type, polarization type, l_trunc, architecture), but the function doesn’t currently validate that contract. A mismatched params can silently produce a model whose solver was derived under different settings than the newly-built quad_points, which is very hard to debug.

Files reviewed: 65/66 changed files
Comments generated: 0 new
Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

cfranken added 6 commits July 10, 2026 14:03

docs(surfaces): document atmosphere/surface split, Lambertian closure…

6e69e11

…, scenario sweeps

Copilot AI lite review requested due to automatic review settings July 11, 2026 00:22

Copilot started reviewing on behalf of cfranken July 11, 2026 00:23 View session

Copilot AI reviewed Jul 11, 2026

View reviewed changes

cfranken self-assigned this Jul 14, 2026

cfranken requested a review from sunitisanghavi July 14, 2026 00:42

feat(absorption): support native ABSCO LUTs

8ded0a5

Copilot AI review requested due to automatic review settings August 4, 2026 23:54

Copilot started reviewing on behalf of cfranken August 4, 2026 23:55 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

fix(absorption): require ABSCO-capable AA release

e8e19c1

Copilot AI review requested due to automatic review settings August 5, 2026 00:57

Copilot started reviewing on behalf of cfranken August 5, 2026 00:58 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

docs(absorption): mark internal engine for retirement

ebc0809

Copilot AI review requested due to automatic review settings August 5, 2026 01:21

Copilot started reviewing on behalf of cfranken August 5, 2026 01:21 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

Navigation Menu

Uh oh!

Atmosphere/surface split: surface sweeps, exact Lambertian albedo closure, scenario batching - #230

Atmosphere/surface split: surface sweeps, exact Lambertian albedo closure, scenario batching#230
cfranken wants to merge 11 commits into
mainfrom
feat/surface-split

cfranken commented Jul 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cfranken commented Jul 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

cfranken commented Jul 11, 2026

What

Heads-up (pre-existing, not fixed here)

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cfranken commented Jul 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Human review recommended

Suppressed comments (1)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Not ready to approve

Suppressed comments (1)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Human review recommended

Suppressed comments (3)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants