Use optimized HDF5 reader for resample_slc_v2 - #372
Merged
Tyler-g-hudson merged 1 commit intoSep 1, 2026
Merged
Conversation
resample_secondary_rslc_onto_reference() read the secondary RSLC via RSLC.getSlcDatasetAsNativeComplex(), which opens the file with h5py's default chunk cache instead of one sized for the dataset's own chunking. Open the secondary RSLC directly via HDF5OptimizedReader instead, matching the pattern already used in unwrap.py, crossmul.py, and other workflows. This cuts the resample step's SLC I/O read time by roughly 5x.
hfattahi
reviewed
Aug 27, 2026
Comment on lines
+223
to
+224
| if is_complex32(dataset): | ||
| dataset = ComplexFloat16Decoder(dataset) |
Contributor
There was a problem hiding this comment.
It's been a long time since we moved on from float 16 for RSLC. We have not produced any NISAR real data RSLC with float 16. I think we don't need this check. Do you agree @bhawkins ?
Contributor
There was a problem hiding this comment.
Is any of our test data float16?
Contributor
Author
There was a problem hiding this comment.
I would recommend keeping the check for now, 1) the original call getSlcDatasetAsNativeCompl offers the same check and conversion, 2) it seems that there are some other places which do the same, such as calls to is_dataset_complex32, read_c4_dataset_as_c8. If we decide to drop complex32, it's better to handle them altogether.
Contributor
There was a problem hiding this comment.
Fine with me. let's keep it.
Tyler-g-hudson
approved these changes
Aug 27, 2026
Tyler-g-hudson
left a comment
Tyler-g-hudson
left a comment
Contributor
There was a problem hiding this comment.
LGTM pending the resolution of Heresh's concern regarding float16.
Contributor
|
Thanks @lijun99 for the contribution! |
sharath-velishala
added a commit
to A-Science-Company/isce3
that referenced
this pull request
Sep 3, 2026
Brings in 5 upstream commits from isce-framework/isce3. None touch asc/, so no conflict with our work. f42cea7 Use optimized HDF5 reader for resample_slc_v2 (isce-framework#372) 23f9932 Solve CI failures (isce-framework#366) fc8112c Update Soil Moisture SAS to v0.4.3 (isce-framework#349) 0d1600d Update STATIC workflow for new water mask spec (isce-framework#334) be7b3d9 Fix failing unit test test.cxx.isce3.io.raster.raster (isce-framework#348) Note these change the FORK's source only. We run isce3 0.25.12 from conda-forge and never build this tree, so none of them affect our runtime -- including isce-framework#372, which touches the resample_slc_v2 that Track R will use. That optimisation lands for us only when conda-forge ships a release containing it.
sharath-velishala
added a commit
to A-Science-Company/isce3
that referenced
this pull request
Sep 3, 2026
…fy.sh tools/apply_patches.py overlays pure-Python upstream fixes onto the released conda-forge package, so we get them without a source build. Building isce3 from source would cost hours, a full compiler toolchain, ~250 GB of scratch, and leave us running an unreleased 0.26.0-dev we then own. First patch: isce-framework#372 (f42cea7), a single Python file. resample_secondary_rslc_onto_reference() opened the secondary RSLC via getSlcDatasetAsNativeComplex(), which uses h5py's DEFAULT chunk cache rather than one sized to the dataset's own chunking. Resampling reads the secondary repeatedly, so an undersized cache re-reads the same chunks from disk. It matters directly for Track R. Three conditions are checked before anything is overlaid: - Python only, no compiled extension involved - every symbol the patched file imports already exists in the installed isce3 (verified: isce3.io.HDF5OptimizedReader and the isce3.core.types helpers are all present in 0.25.12) - the installed file is byte-identical to the pre-patch upstream version, so we know exactly what is being replaced The original is kept as <name>.orig and restored by --revert. verify.sh now reports patch status with --check; it never modifies the environment itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
3 participants
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.
resample_secondary_rslc_onto_reference() read the secondary RSLC via RSLC.getSlcDatasetAsNativeComplex(), which opens the file with h5py's default chunk cache instead of one sized for the dataset's own chunking. Open the secondary RSLC directly via HDF5OptimizedReader instead, matching the pattern already used in unwrap.py, crossmul.py, and other workflows. This cuts the resample step's SLC I/O read time by ~5-10x. Output is verified bit-for-bit identical to the original.
Moved from #358.