Make quality_mask safe for the EMIT L2A Mask V002 product - #72
Open
thc1006 wants to merge 2 commits into
Open
Conversation
quality_mask hard-coded bands 5 and 6 (the V001 AOD/H2O data bands) as the
only non-flag layers. The V002 mask adds three continuous layers --
SpecTf-Cloud Probability and SpecTf-Buffer Distance -- and reorders the flags,
so requesting one of the new data bands was silently treated as a binary flag:
its values were summed and clipped to {0, 1}, producing a semantically
meaningless mask.
Classify each requested layer from its sensor_band_parameters/mask_bands name
instead. There is no separate flag-vs-data metadata field in either product, so
the band name is the only per-layer discriminator:
- binary flags ("... Flag") are OR-combined, exactly as before;
- continuous data layers (AOD550, H2O, SpecTf-Cloud Probability,
SpecTf-Buffer Distance) are rejected with an error naming the band;
- a probability layer can be turned into a mask via the new optional
threshold argument (probability >= threshold);
- band indices are validated and the returned mask is asserted to be {0, 1}.
Verified against a real EMITL2AMASK V002 granule: the binary-flag path is
value-identical to the previous implementation, while the new SpecTf data
bands are now rejected instead of silently accepted.
Add pytest regression tests covering the V001 (8-band) and V002 (11-band)
layouts, including the previously-silent failure.
Co-authored-by: junnncct1106 <jun.514114.ee10@nycu.edu.tw>
… stricter validation
Follow-up to the V002 metadata-driven quality_mask fix, addressing review
feedback:
- Non-finite (fill / no-data) pixels are now fail-closed: any NaN/+Inf/-Inf is
excluded (mask = 1), never returned as 0 (clear), in both the flag path and
the threshold path. The mask variable declares _FillValue = -9999, which
xarray decodes to NaN, so this is a real product concern.
- Open the mask dataset and sensor-band group in a `with` block so both file
handles are always closed, including on error (the original code leaked them).
- Reject boolean indices explicitly (bool is a subclass of int) and validate
that the mask band axis length matches the number of mask_bands names.
- Use named indexing (isel) and incremental OR; the result is uint8 {0, 1} by
construction, so the redundant final assert was removed.
- Docstring: add a V002 note (11 bands, reordered indices, print mask_bands
rather than hard-coding); add the same note to the quality how-to notebook.
Tests: switch the synthetic fixtures to the xarray/h5netcdf backend (no netCDF4
test dependency), add a requirements-test.txt, and add cases for non-finite
(NaN, +/-Inf, and a declared _FillValue) fail-closed behavior, boolean and
axis-mismatch rejection, and dataset closure on success and error. The conftest
now stubs only the specific missing optional dependency instead of masking any
import error.
Co-authored-by: junnncct1106 <jun.514114.ee10@nycu.edu.tw>
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.
Summary
quality_maskinpython/modules/emit_tools.pysilently produces a wrong mask for the EMIT L2A Mask V002 product. It hard-codes bands5and6(the V001AOD550/H2Odata bands) as the only non-flag layers:The V002 mask adds continuous layers and reorders the flags, so any V002 continuous band other than 5/6 falls through to the
elsebranch and is treated as a binary flag: its values are summed and clipped>1 -> 1. The result is not a{0, 1}mask, and downstreamqmask == 1masking then behaves incorrectly, with no error raised.Verified V002 band layout
Confirmed by reading
sensor_band_parameters/mask_bandsfrom a real granule (EMIT_L2A_MASK_002_20220810T034103_2222203_001.nc,EMITL2AMASKv002). V002 has 11 bands: 7 binary flags and 4 continuous data layers.Cloud FlagCirrus FlagWater FlagSpacecraft FlagDilated Cloud FlagAOD550H2O (g cm-2)Aggregate FlagSpecTf-Cloud ProbabilitySpecTf-Cloud FlagSpecTf-Buffer DistanceThe flags were reordered relative to V001 (e.g.
Dilated Cloud Flagmoved to index 4), so band indices are not interchangeable between versions and index-based logic is fragile.Reproduction (silent failure)
On the real granule the current code returns a
float32array with 31 distinct values in the range [0.2637, 1.0] (30 of them fractional) instead of a{0, 1}mask. It never raises.SpecTf-Buffer Distance(index 10) is similarly accepted; it happens to be0/1in this scene but is a pixel-distance layer that will hold values>1in cloudier scenes and be silently clipped.Fix
There is no explicit "is-a-flag" metadata field in the mask file (the
sensor_band_parametersgroup has no such attribute, and the singlemaskvariable shares oneunits), so the band name is the only per-layer discriminator.quality_maskis now metadata-driven and hardened:sensor_band_parameters/mask_bandsand classifies each requested layer by name;Flag(case-insensitive, matching both V001 and V002);AOD550,H2O (g cm-2),SpecTf-Cloud Probability,SpecTf-Buffer Distance) with aValueErrorthat names the band and lists the file's actual flag bands;thresholdargument so a probability layer can be turned into a mask explicitly (probability >= threshold);Flag-named layer whose finite values are not binary rather than clipping it.For binary flags the combination is a logical OR, which reproduces the previous sum-then-clip result exactly.
Non-finite (fill / no-data) handling — fail-closed
The
maskvariable declares_FillValue = -9999, which xarray's defaultmask_and_scaledecodes to NaN, so a layer can contain non-finite (no-data) pixels. The real granule above contains no non-finite pixels, but fill/no-data is a documented part of the product schema, so the code must handle it. A no-data pixel is not a clean observation, so the mask is now built fail-closed: any non-finite value is excluded (mask = 1), never returned as0(clear). This applies to both the flag path and thethreshold(probability) path.Concretely, for a flag layer with pixel values
[0, 1, NaN, +Inf, -Inf],quality_masknow returns[0, 1, 1, 1, 1], whereas the naivelayer > 0returned[0, 1, 0, 1, 0](NaN and-Infsilently treated as clear). The threshold path behaves the same way.Resource management, and additional validation
quality_maskopens are now opened in awithblock, so both file handles are always closed, including when the function raises (the previous code, and upstream, leaked them). The returned array is a detached numpy array, so closing is safe.boolis a subclass ofint, andnp.bool_would otherwise be used as a boolean index deep inside xarray).mask_bandsnames raises a clearValueErrorinstead of a laterIndexErroror a mis-slice.Verification on the real granule
Running the previous function and the new one side by side on
EMIT_L2A_MASK_002_20220810T034103_2222203_001.nc:[0, 1, 4]-> same{0, 1}mask,1588851pixels masked in both).8(SpecTf-Cloud Probability): old returns the non-binary float array described above; new raisesValueError: Band 8 ('SpecTf-Cloud Probability') is a continuous data layer, not a binary quality flag ....10(SpecTf-Buffer Distance): new raises naming the band.quality_mask(fp, [8], threshold=0.5)returns auint8{0, 1}mask equal to a directly-computednumpy.where(isfinite, raw >= 0.5, True)reference.5/6remain rejected (nowValueErrornamingAOD550/H2O (g cm-2)).Backward compatibility
quality_mask(filepath, quality_bands, threshold=None); existing two-argument calls are unchanged, and legacy V001 flag combinations return the same mask as before.ValueError(wasAttributeError) with a clearer, band-naming message.uint8{0, 1}(wasfloat32{0.0, 1.0}); finite values are unchanged andqmask == 1masking is unaffected.Tests
Adds
python/modules/tests/(network-free pytest) with small synthetic V001 (8-band) and V002 (11-band) mask files whosemask_bandsmetadata mirrors the real products. Coverage: legacy V001 flags still combine correctly, V002 binary flags combine correctly, continuous V002 bands now raise instead of silently returning a wrong mask, thethresholdpath, non-finite handling (NaN,+Inf,-Inf, and a declared_FillValue) is fail-closed, bool/out-of-range/axis-mismatch indices raise, and the opened datasets are closed on both the success and error paths. An optional test runs against a real granule whenEMIT_L2A_MASK_V002points to one and checks thethresholdresult against a directly-computed reference.The fixtures use only xarray + h5netcdf (already required by
emit_tools), so the tests add no netCDF backend dependency;python/modules/tests/requirements-test.txtlists the test dependencies (pytest is the only addition). The tests stubemit_tools' optional heavy geospatial imports (GDAL, geopandas, rasterio, ...) when they are absent so the suite runs in a minimal environment, and stub only the specific missing module so a real import/ABI failure is not masked.A short note describing the V002 mask layout was added to
python/how-tos/How_to_use_EMIT_Quality_data.ipynb.