Skip to content

fix: float comparison in Sentinel-2 saturation - #12

Merged
ceholden merged 3 commits into
mainfrom
fix/sentinel2-saturation
Nov 5, 2026
Merged

ceholden merged 3 commits into
mainfrom
fix/sentinel2-saturation

Conversation

@ceholden

@ceholden ceholden commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Closes https://github.com/NASA-IMPACT/hls_development/issues/441

The Fmask code from GERSL has no library versions included in the installation details, but anecodtally it seems like it was developed on very old versions, including numpy<2. NumPy v2 was released in June, 2024, so anyone following their conda-based install details in recent years would get a version of NumPy v2.

This major version difference is problematic because of a change in NumPy v2 regarding type promotion rules following NEP-50. The code does a comparison of band data in float32 against a (Python-dtype) float, so in NumPy v2 a type promotion occurs and saturation comparison checks fail.

To give a brief example:

cube_value = np.float32(6.4535)  # stored in the float32 cube, rounded
scalar = np.float64(6.4535)      # the comparison value, not rounded

cube_value == scalar
# NumPy 1: scalar is cast down to float32, so True
# NumPy 2: cube_value is widened to float64, keeps its rounding error, so False

This doesn't happen in the Landsat code path because the saturation check was done in units of DNs (integers, before scaling) rather than in (float) units of TOA reflectance.

What I did

I fixed this by mimicking the Landsat code path:

  1. Add saturation: bool = False to the Sentinel2.read_band method and when True return a boolean array of the saturation check done in units of DNs
  2. Run the saturation check for the 3 bands used for Sentinel-2 checks during read_band calls

I added unit tests for this use case and set up the repo to run the unit tests in CI for PRs and merges to main.


Once this is merged I'll create an issue upstream and perhaps include a PR if the merge isn't too gnarly. Alas the GERSL repository has switched off of the main branch and created an entirely new Git history in their develop branch, making synchronization more difficult than it needs to be.

ceholden and others added 3 commits November 4, 2026 15:39
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
read_datacube detected saturation by comparing the float32 cube with the float64
TOA value of the saturated DN. NumPy 1 cast that scalar to float32; NumPy 2
compares in float64, where the rounded band values no longer match, so saturated
pixels were not flagged. The saturation layers feed whiteness, the potential
cloud test, the absolute snow test and the NDVI/NDSI fix in the land variability
probability.

Compute saturation from the DN inside read_band instead, so no float precision
is involved. Sentinel2.read_band now takes the same arguments as Landsat.read_band
and likewise returns the image with its saturated pixels. This changes masks on
scenes with saturated visible pixels when running NumPy 2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Installs the locked test environment with pixi on Linux, so a pull request also
fails if pixi.lock has drifted from pixi.toml.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ceholden
ceholden added this pull request to stack #14 November 4, 2026 22:16
@ceholden
ceholden merged commit db718cb into main Nov 5, 2026
1 check passed
@ceholden
ceholden deleted the fix/sentinel2-saturation branch November 5, 2026 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant