-
Notifications
You must be signed in to change notification settings - Fork 32
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
Specify number of cores for jump and ramp_fit #183
Specify number of cores for jump and ramp_fit #183
Conversation
For some users on large machines even using 'quarter' leads to them using too many cores. This change allows string integer values to be used.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #183 +/- ##
==========================================
+ Coverage 74.11% 75.36% +1.24%
==========================================
Files 33 33
Lines 6100 6039 -61
==========================================
+ Hits 4521 4551 +30
+ Misses 1579 1488 -91
☔ View full report in Codecov by Sentry. |
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.
I'd like to request the ramp fit test be moved to a different location per my specific comment above. Other than that, everything looks good.
The CI failures are caused by unit tests in the jwst/ramp_fitting package trying to load the now obsolete function |
OK, this is looking better. Just need to address the 2 errors coming from the code style CI test (removed unused imports from test modules). |
Still need to have the style check errors fixed before we can merge: tests/test_jump.py:3:24: F401 [] |
remove unused import
remove unused import
I've fixed the code style errors. |
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.
Finally all looks good.
This PR addresses the problem that for multiprocessing the number of cores to use can only be a fraction of the total number of cores. This is fine for small machines but on large clusters even 1/4 can be larger than someone is allowed to use.
The PR adds the flexibility to specify explicitly the number of cores the user wants to use. Changes were made for both Jump and Ramp fitting and new methods were written with unit tests.
No code changes are needed in the JWST repo only in the documentation.
Checklist
CHANGES.rst
(either inBug Fixes
orChanges to API
)