Add atl03_signal_class photon-selection parameter (ATL03 R007 signal_class_ph) to atl03x

[dshean]

Stacked on #649 — the first two commits here are that PR (the fix and its review-response commit); the feature is the last two commits. If #649 merges first, this PR rebases to feature-only; happy to do that on request.

Summary

ATL03 R007 ships an experimental per-photon classification /gtx/heights/signal_class_ph, derived from the YAPC weight_ph weights (−1 ignored, 0 likely_noise, 1 likely_signal, 2 signal_below, 3 signal_above, 4 primary_signal, 5 fitted_signal; primary/fitted mark the highest reflecting surface found per ~5 m fit segment). atl03_ph_fields can already deliver the column, but nothing can select on it. This PR adds an opt-in atl03_signal_class parameter mirroring the atl08_class machinery on atl03x — inert unless supplied, no change to any default behavior — so that e.g.

{"atl03_signal_class": ["primary_signal", "fitted_signal"], "fit": {}}

produces a server-side surface fit to the granule's own first-reflecting-surface photon selection, with no ATL08 dependency and no threshold to tune.

Motivation and quantitative evaluation — why a scalar yapc.score threshold cannot cover brightness-varying scenes, and how this selection performed in a 383-fit-window seasonal-snow study — are in the companion issue #650.

Semantics and limitations (measured; details in #650)

The selection is first-surface by construction. Over exposed ground a fit on it is simply a surface fit; over snow-free dense conifer forest the first surface is the canopy, so a ground-style fit is biased high there (+0.89 m median in the evaluation's snow-off steep-forest windows — users wanting snow-off forest terrain should use atl08_class: ["atl08_ground"] instead), and conversely a robust fit on this selection is not a canopy-top (DSM) product. signal_class_ph is new in R007 and marked experimental, so the parameter fails loudly rather than silently: requests against pre-R007 granules raise a clear alert, and the legacy reader rejects the parameter as unsupported.

Implementation

• accepts classification names ("primary_signal", ...) or the numeric values −1..5;
• signal_class_ph is read only when the parameter is supplied (it is photon-rate, so the read is conditional, like the ATL08 companion reads);
• the filter applies in the photon selection loop alongside cnf, quality_ph, atl08_class, and yapc, i.e. before fit/phoreal;
• the selected photon's classification is returned in a new atl03_signal_class column (int8);
• requests against pre-R007 granules raise a clear alert (the dataset does not exist there), and the legacy Atl03Reader rejects the parameter as unsupported instead of silently ignoring it;
• a follow-up commit enables the optional column in the representative OpenAPI schema-generation request, so the generated schema documents it (the parameter itself is picked up automatically from addParameter metadata).

Files changed

File	Change
apps/node/datasets/icesat2/package/Icesat2Parameters.{h,cpp}	atl03_signal_class_t enum + Lua/JSON/index converters (pattern of atl08_class_t; index offset like signal_conf_t for the −1 value)
apps/node/datasets/icesat2/package/Atl03Parameters.{h,cpp}	atl03SignalClass field + atl03_signal_class parameter registration
apps/node/datasets/icesat2/package/Atl03DataFrame.{h,cpp}	conditional signal_class_ph read; release guard; selection-loop filter; atl03_signal_class column
apps/node/datasets/icesat2/package/Atl03Reader.cpp	reject atl03_signal_class on legacy endpoints
apps/node/scripts/openapi.lua	enable the optional column in the representative schema-generation request
apps/node/datasets/icesat2/selftests/parameters.lua	parse tests: names + numeric values; invalid name/value falls back to an empty (inert) selection with a server-side warning, matching the FieldMap convention (offline)
apps/node/datasets/icesat2/selftests/atl03_dataframe.lua	cloud tests: selection reduces rows, returns only selected classes, adds column; pre-R007 rejected; fit-on-selection returns ≤ unfiltered fit rows
clients/python/tests/test_atl03x.py	same behaviors against a live endpoint
docs/rtd/source/user_guide/icesat2.md	new §1.2.5 ATL03 Signal Classification; column tables

Testing

Built and tested locally in the project buildenv container (arm64, amazonlinux:2023, per the ARM dev-environment setup): release build compiles clean with no warnings or errors, and make selftest passes with 0 errors (76 tests, 907 asserts across the offline suite, including the new parameters.lua tests; the cloud-gated atl03_dataframe.lua tests for this feature skip locally by design and need an in-cloud run). The live-endpoint Python tests are written but require a deployed server carrying this branch. The evaluation numbers referenced in #650 were produced by emulating the selection client-side on atl03_ph_fields ["weight_ph","signal_class_ph"] pulls and applying the identical fit — i.e., they validate the selection semantics this PR exposes, on real R007 granules.

Related

• Issue yapc.score threshold is silently collapsed to a 0/1 test by an implicit FieldElement bool conversion (atl03x and legacy endpoints); yapc v1-3 on atl03x silently return all-zero scores #648 (inert yapc.score; its final section records this feature ask) and fix PR Fix inert yapc.score photon filtering and fail loudly on unsupported YAPC configurations (fixes #648) #649 — this branch is stacked on that fix.
• Closes Feature: atl03_signal_class photon-selection parameter for atl03x (select on ATL03 R007 signal_class_ph) #650

[jpswinski]

@dshean Is there a driving need to a server side signal_class_ph filter? Could this be accomplished via supplying it as an ancillary field in the request and then filtering on the client (web or Python)?

[dshean]

The idea was to use these during the server-side ATL06 fits, instead of the 'ground' classified photons. Basically another way to customize which photons are going into the ATL06 processor.

[jpswinski]

@dshean I see, that makes sense. Then the change I would want to see in how it is coded is for the enumeration field to default to everything being true so that when the request parameters are represented in the returned dataframe metadata, the filter is correctly captured. Then the code should check to see if anything is disabled and if anything is disabled, then it can conditionally read the signal class ph variable in the hdf5 file.

[Copilot]

Pull request overview

Adds a new atl03_signal_class photon-selection parameter to the ICESat-2 atl03x path, enabling server-side filtering on ATL03 R007’s experimental signal_class_ph classifications (and returning the selected class as an output column). This PR also includes the stacked fix work that makes yapc.score filtering effective and ensures unsupported YAPC configurations fail loudly instead of silently degrading results.

Changes:

• Add atl03_signal_class parameter parsing/serialization (Lua/JSON/index) and integrate selection into the ATL03 photon subsetting loop with R007 release guards.
• Add atl03_signal_class as an opt-in output column and update OpenAPI schema generation to include it.
• Add/extend offline + cloud-gated Lua selftests and live-endpoint Python tests; update user guide documentation for new selection and clarified YAPC score scaling.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
docs/rtd/source/user_guide/icesat2.md	Documents atl03_signal_class semantics/limits and clarifies yapc_score scale by ATL03 release/source.
clients/python/tests/test_atl03x.py	Adds live-endpoint tests for YAPC score filtering, YAPC unsupported versions, and signal-class filtering/fit behavior.
apps/node/scripts/openapi.lua	Enables the optional atl03_signal_class column in the representative schema-generation request.
apps/node/datasets/icesat2/selftests/parameters.lua	Adds offline parameter-parse tests for atl03_signal_class (names, numeric values, invalid handling).
apps/node/datasets/icesat2/selftests/atl03_dataframe.lua	Adds cloud-gated behavioral tests for YAPC score filtering, unsupported YAPC versions, and signal-class filtering + fit interaction.
apps/node/datasets/icesat2/package/Icesat2Parameters.h	Introduces atl03_signal_class_t enum and converter declarations/encoding hook.
apps/node/datasets/icesat2/package/Icesat2Parameters.cpp	Implements enum converters for atl03_signal_class_t (Lua/JSON/index).
apps/node/datasets/icesat2/package/Atl03Parameters.h	Adds atl03SignalClass field (enumeration) to ATL03 parameters.
apps/node/datasets/icesat2/package/Atl03Parameters.cpp	Registers atl03_signal_class; adds parse-time validation for yapc.version.
apps/node/datasets/icesat2/package/Atl03DataFrame.h	Adds atl03_signal_class column, signal_class_ph dataset handle, and useSignalClass flag.
apps/node/datasets/icesat2/package/Atl03DataFrame.cpp	Conditionally reads signal_class_ph, guards pre-R007 usage, filters photons by class, and fixes YAPC score comparisons to use .value.
apps/node/datasets/icesat2/package/Atl03Reader.cpp	Rejects atl03_signal_class on legacy endpoints and fixes YAPC score comparisons to use .value (plus pre-R006 guard).

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[dshean]

In-cloud verification of this branch on a us-west-2 Graviton instance (Amazon Linux 2023, sliderule-buildenv:latest container, instance-role auth): full selftest suite = 76 tests, 20 skipped, 4029 asserts. All icesat2 cloud selftests pass, including the new atl03_signal_class cases in atl03_dataframe.lua: on ATL03_20200304065203_10470605_007_01 gt1l over the test polygon the selection reduces 15187 photons to 14161, only classes 4/5 come back, and the pre-007 granule is rejected with zero photons.

On the question above: the new ATL06 surface-fitter selftest exercises exactly that server-side use, the fitter consuming the signal-class-selected photons. On this granule/polygon it produces 524 elevations from the 14161 selected photons, versus 415 from all 15187 photons at cnf=0 and 527 from the 13317 photons at cnf=4.

One assertion in that test was wrong and is fixed in 258591b: it assumed the fitter returns no more elevations from the signal-class-selected photons than from the unselected set. Fit row counts are not monotonic in photon count (noise photons make segments fail the robust fit), as the cnf=4 vs cnf=0 numbers above show for the existing filter. The test now checks that the selection changes the fit result instead of asserting a direction.

[dshean]

Done in d624900: atl03_signal_class now defaults to every classification enabled, so the request parameters recorded in the dataframe metadata carry the full list when no filter is in effect. The reader checks anyDisabled() and only then reads signal_class_ph (and adds the atl03_signal_class column); the legacy atl03s check uses the same test, so the default no longer trips it. Selecting all seven classes explicitly is equivalent to the default (covered by a new selftest case), and the parameter description and user guide say so.

One consequence worth a look: an invalid classification name now leaves the enumeration empty (fromLua clears it before converting, and FieldMap's catch keeps that state), so the filter is active and excludes every photon with only a server-side WARNING. That is the same behavior quality_ph has today, so I left it consistent rather than special-casing it; happy to make it a hard rejection instead if you prefer.

Verified on the us-west-2 instance: parameters.lua, atl03_dataframe.lua and atl03_reader.lua pass in cloud context, and the full selftest suite is unchanged apart from the environmental cases noted earlier.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

apps/node/datasets/icesat2/selftests/atl03_dataframe.lua:224

• The test’s min_score_* assertions become vacuous if count_rows returns 0 rows for score=1000 or score=8000: min_score stays at the sentinel value (65535) and still satisfies min_score >= threshold. To make this test actually validate the filtering behavior, add explicit assertions that rows_mid > 0 and rows_high > 0 (or compute min_score only when df:numrows() > 0 and assert accordingly).

        local min_score = 65535
        for i = 1, math.min(df:numrows(), 1000) do
            if df["yapc_score"][i] < min_score then
                min_score = df["yapc_score"][i]
            end
        end
        return df:numrows(), min_score
    end

    local rows_all, min_score_all = count_rows(0)
    local rows_mid, min_score_mid = count_rows(1000)
    local rows_high, min_score_high = count_rows(8000)

    runner.assert(rows_all > 0, "no photons returned for score 0")
    runner.assert(rows_mid <= rows_all, string.format("score 1000 did not filter photons: %d > %d", rows_mid, rows_all))
    runner.assert(rows_high < rows_all, string.format("score 8000 did not filter photons: %d >= %d", rows_high, rows_all))
    runner.assert(rows_high <= rows_mid, string.format("score threshold not monotonic: %d > %d", rows_high, rows_mid))
    runner.assert(min_score_mid >= 1000, string.format("photon below score threshold returned: %d < 1000", min_score_mid))
    runner.assert(min_score_high >= 8000, string.format("photon below score threshold returned: %d < 8000", min_score_high))