JP-2623: Fixing multiprocessing failure #99
Conversation
Codecov Report
@@ Coverage Diff @@
## main #99 +/- ##
==========================================
+ Coverage 71.65% 72.11% +0.46%
==========================================
Files 16 16
Lines 2466 2478 +12
==========================================
+ Hits 1767 1787 +20
+ Misses 699 691 -8
Continue to review full report at Codecov.
|
|
@kmacdonald-stsci can you fix the conflicts, which will then allow the CI tests to run again. |
kmacdonald-stsci
force-pushed
the
jp_2623
branch
from
584766b
to
b4aa41e
Compare
|
@dmggh are you OK with this PR? |
|
any way we can get this PR and the related one on JWST merged so we have multiprocessing available in master soonish ? |
|
This also includes a minor, but noticeable, change to the way some cases of one good group are handled, which we were waiting for comment and testing from INS before merging. So we are not going to include this in our B8.0.1 release (which is being created today). Once the B8.0.1 release has been created we can then merge this anytime we want in order to get it into master. |
…age is used in order to be usable with multiprocessing. Adding NoneType conditional to slicing up ZEROFRAME for multiprocessing. Removed problematic variables. Multiprocessing now works. Modifying how one group ramps are suppressed. Changing name of function to better describe its functionality. Removing int_times from ramp fitting. Making changes due to style failures. Updating the change log. Adding check for keyword in exposure metadata.
kmacdonald-stsci
force-pushed
the
jp_2623
branch
from
93c00c0
to
a2ec4ee
Compare
|
I have used this branch and the accompanying |
|
@PaulHuwe @ddavis-stsci Can one of you run a test with romancal to make sure the absence of int_times in the return values does not break romancal? It'd be good to merge this soon if possible. |
|
@kmacdonald-stsci Please assign also @ddavis-stsci and @PaulHuwe as reviewers of all stcal PRs to cover Roman testing. |
There was a problem hiding this comment.
LGTM, the roman regression tests pass with Ken's changes,
https://plwishmaster.stsci.edu:8081/me/my-views/view/all/job/RT/job/Roman-Developers-Pull-Requests/48/
|
Given all the glowing reports from MIRI team members and Roman testers, I feel we can go ahead and merge this. Any objections? Speak now or ... |
The multiprocessing feature of ramp fitting was failing. To ensure new features worked properly with multiprocessing the unneeded one group suppression list got removed and the
ZEROFRAMEhandling got moved to after multiprocessing to ensure proper usage.The primary cause of the multiprocessing failure was the
int_timesobject, which is anastropy.ioobject that cannot be properly pickled. Since it is not used in ramp fitting, it is no longer passed toSTCALramp fitting.