Fix inert yapc.score photon filtering and fail loudly on unsupported YAPC configurations (fixes #648)

[dshean]

Root cause

The photon-selection threshold documented for yapc.score has been
silently inert on both atl03x and the legacy reader (#648). The filter
code exists and looks correct:

if(yapc_score < parms.yapc.score)   // Atl03DataFrame.cpp (2 sites)
if(yapc_score < parms->yapc.score)  // Atl03Reader.cpp (3 sites)

but score is a FieldElement<uint16_t>, and FieldElement defines
operator bool() const { return value != 0; } and no operator T().
Overload resolution therefore converts the field object to bool,
and the comparison is against 0 or 1 — never the requested threshold.
Any non-zero score behaves as "score >= 1", which explains every
symptom reported in #648:

• atl03x + {version: 0, score: N}: identical results for N = 0/150/
  1000/3300/8000, minimum returned yapc_score = 1 (weight-0 photons
  are typically already excluded by the default quality_ph filter);
• legacy atl06p + {version: 3, score: N}: selection changes once when
  any N > 0 is supplied (score-0 photons dropped) and is then
  insensitive to the value of N.

Additionally, {version: 1..3} on atl03x silently returned all-zero
scores (no v1-3 implementation exists in the dataframe path and nothing
rejected the request), and {version: 0} against pre-R006 granules
(no weight_ph dataset) silently returned zero scores on both paths.
With the comparison fixed, that last case would have become worse —
a non-zero threshold against all-zero scores silently returns an empty
result — so it now errors instead.

What changed

• Atl03DataFrame.cpp / Atl03Reader.cpp: the five comparison sites
  use parms.yapc.score.value (the same pattern the neighboring ATL24
  confidence filter already uses).
• Atl03DataFrame.cpp (atl03x): requests with yapc.version != 0 and
  requests with version == 0 against pre-R006 granules now throw,
  which surfaces as a CRITICAL alert on the response stream instead of
  silent zero scores.
• Atl03Reader.cpp (legacy atl03s/sp, atl06/p): YapcScore now throws
  for version == 0 against pre-R006 granules (v1-3 already threw for
  invalid versions).
• Atl03Parameters.cpp: yapc.version outside 0-3 is rejected at
  parameter parse time.
• docs/rtd/source/user_guide/icesat2.md: documents the score scale
  (below) and the loud rejection of v1-3 on atl03x.
• Tests (below).

Semantics decision: v0 score scale

With version: 0 the scores are the granule weight_ph values read
verbatim, and the threshold is compared against those raw values:
0-65535 for R007 granules (DDA-03 saturation-normalized weights;
65535 = saturation density), 0-255 for R006. Server-computed v1-3
scores remain 0-255. I kept the raw-scale comparison rather than
normalizing because (a) the returned yapc_score column is the raw
value, so threshold and column stay directly comparable, (b) any
rescaling would silently change meaning again — the failure mode this
PR removes — and (c) the R007 scale is the product's own documented
scale (ATL03 ATBD R007 §5.2). The docs previously stated 0-255
unconditionally; they now state the scale per source. (Happy to switch
to a normalized comparison if you'd rather keep the documented 0-255
interface stable across releases — it's a two-line change on top of
this.)

Tests

• selftests/parameters.lua (offline): invalid yapc.version rejected
  at parse.
• selftests/atl03_dataframe.lua (cloud): score threshold monotonically
  reduces row counts and enforces the minimum returned score on an R007
  granule already used by the repo's selftests
  (ATL03_20200304065203_10470605_007_01.h5); {version: 3} on atl03x
  returns an empty dataframe with an alert. Assertions are relational
  (monotonic counts, min-score bounds) rather than magic row counts, so
  they are robust to granule reprocessing. No existing expected values
  were touched.
• clients/python/tests/test_atl03x.py (live endpoint): the same two
  behaviors through the Python client, against the R007 twin of the
  existing test granule (ATL03_20181019065445_03150111_007_01.h5).

Testing performed vs. deferred to CI

Honest status: I could not build the server locally (macOS box without
the buildenv container). What I did run:

• clang++ -std=c++20 -fsyntax-only over every modified C++ file plus
  Icesat2Parameters.cpp, Atl06DispatchParameters.cpp, and
  SurfaceFitter.cpp as consumers, against the repo headers (Linux
  timer_t/UUID_STR_LEN/BUILDINFO shimmed) — clean;
• luac -p on both modified selftests; python -m py_compile and
  codespell (repo pre-commit config) on the Python test and docs.

Not run: make selftest (needs a server build; the new
parameters.lua case is offline and will run there), the cloud
selftest, and the Python client tests (need a deployed build with this
change). I'd rely on your testrunner for those; the failure signatures
in #648 were verified empirically against the public v5.5.0 cluster,
and the new tests assert exactly the behaviors that were observed
broken.

Possible follow-up (not in this PR)

Making FieldElement::operator bool() explicit would have turned all
five silent-conversion sites into compile errors and prevents
recurrence; I left it out to keep this diff minimal since I could not
compile the full tree to chase the fallout. Can open a separate issue
if useful.

Fixes #648

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a long-standing bug where yapc.score filtering was effectively reduced to a boolean (0/1) check due to implicit FieldElement conversions, and it adds explicit (loud) failures for YAPC configurations that atl03x cannot support or for granules that cannot supply v0 scores.

Changes:

• Fix YAPC score threshold comparisons by using the underlying .value for FieldElement<uint16_t> at all filtering sites.
• Reject unsupported YAPC configurations (e.g., atl03x with version != 0, and v0 requests on pre-R006 granules with no weight_ph scores).
• Update ICESat-2 user docs and add selftests + Python client tests to cover the corrected behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
docs/rtd/source/user_guide/icesat2.md	Updates documented yapc_score scale and atl03x support/rejection semantics.
clients/python/tests/test_atl03x.py	Adds live-endpoint regression tests for v0 score threshold filtering and unsupported versions.
apps/node/datasets/icesat2/selftests/parameters.lua	Adds offline selftest ensuring invalid yapc.version is rejected during parameter parsing.
apps/node/datasets/icesat2/selftests/atl03_dataframe.lua	Adds dataframe selftests validating threshold monotonicity and unsupported-version behavior.
apps/node/datasets/icesat2/package/Atl03Reader.cpp	Fixes legacy reader score filtering to use .value and errors when v0 scores aren’t available.
apps/node/datasets/icesat2/package/Atl03Parameters.cpp	Rejects yapc.version outside 0–3 at parse time.
apps/node/datasets/icesat2/package/Atl03DataFrame.cpp	Fixes atl03x score filtering to use .value and fails loudly on unsupported configs/missing v0 scores.

---

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