-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RCAL-702: Fix memory use issue for Cas22 jump detection #226
RCAL-702: Fix memory use issue for Cas22 jump detection #226
Conversation
This significantly reduces memory usage ~150Gb -> 1Gb
Codecov ReportAll modified and coverable lines are covered by tests ✅
📢 Thoughts on this report? Let us know!. |
bf224da
to
c6f90be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks!
@stscieisenhamer , mind checking to see if this resolves your issue? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dramatic change in memory usage: From 150GB to not even breaking 1GB.
And also, there is an actual run-time difference in running with and without --use_jump_detection
. Without the step runs ~1.5 minutes. With jump detection, the task runs about as long as before this fix, ~6 minutes. As a reminder, before the runtime was no different between the two.
LGTM
I think I see that with include_diagnostic=False we don't get any information back about where the jumps are and we don't end up updating the DQ array. I think we need that. Ideally that could happen in place on the DQ array, so no additional memory were needed; I'm not sure how convenient that ends up being here. |
Resolves RCAL-702
This PR resolves a massive memory spike as part of the solution to RCAL-702
Checklist
CHANGES.rst
(either inBug Fixes
orChanges to API
)