Implement dynamic memory allocation for multi-frame AutoRIFT jobs#3018
Open
cmspeed wants to merge 3 commits intoASFHyP3:developfrom
Open
Implement dynamic memory allocation for multi-frame AutoRIFT jobs#3018cmspeed wants to merge 3 commits intoASFHyP3:developfrom
cmspeed wants to merge 3 commits intoASFHyP3:developfrom
Conversation
1780d15 to
124e037
Compare
124e037 to
d46c192
Compare
jhkennedy
reviewed
Feb 19, 2026
Contributor
jhkennedy
left a comment
There was a problem hiding this comment.
Looks good! Just a couple of minor tweaks that should probably be made
Comment on lines
+39
to
+40
| frame_counts = [len(job_parameters.get(k) or []) for k in ['granules', 'reference', 'secondary']] | ||
| num_frames = max(frame_counts) |
Contributor
There was a problem hiding this comment.
You don't need to check granules here as the frame count will always be 2 for it (and it's deprecated), and not checking it will make expectations/troubleshooting clearer:
https://github.com/ASFHyP3/hyp3/blob/develop/job_spec/ARIA_AUTORIFT.yml#L78-L79
Suggested change
| frame_counts = [len(job_parameters.get(k) or []) for k in ['granules', 'reference', 'secondary']] | |
| num_frames = max(frame_counts) | |
| frame_counts = [len(job_parameters.get(k) or []) for k in ['reference', 'secondary']] | |
| num_frames = max(frame_counts) |
| { | ||
| 'job_type': 'AUTORIFT', | ||
| 'job_parameters': { | ||
| 'granules': ['S2_stub'], # Used for sensor detection |
Contributor
There was a problem hiding this comment.
Suggested change
| 'granules': ['S2_stub'], # Used for sensor detection |
You shouldn't need this line because get_granules grabs all the reference and secondary scenes:
https://github.com/ASFHyP3/hyp3/blob/develop/apps/set-batch-overrides/src/set_batch_overrides.py#L32-L37
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.
This PR updates the
AUTORIFTjob-type memory allocation logic to support multi-frame processing (e.g., earthquake coseismic displacement jobs covering large surface ruptures) without requiring a new job type.Key Changes
Sentinel-2 jobs containing more than 4 frames (in either
referenceorsecondary) are now allocated 16GB memory. Sentinel-2 jobs containing 4 or fewer frames remain at 8GB.Replaced sensor-specific memory constants (e.g.,
AUTORIFT_S2_MEMORY) with general, capacity-based tiers (e.g.,AUTORIFT_MEMORY_8GB,AUTORIFT_MEMORY_16GB,AUTORIFT_MEMORY_32GB).Pre-existing memory allocation remains unchanged, with the exception of name changes to memory allocation constants (see above bullet). Specifically, single-frame Landsat jobs and all Sentinel-1 jobs default to 16GB and 32GB, respectively.