Fix for jumps in first good group after dropping groups #84
Conversation
Codecov Report
@@ Coverage Diff @@
## main #84 +/- ##
===========================================
- Coverage 88.80% 71.79% -17.02%
===========================================
Files 16 16
Lines 2305 2308 +3
===========================================
- Hits 2047 1657 -390
- Misses 258 651 +393
Continue to review full report at Codecov.
|
|
@mwregan2 I think the problem with the new unit test is the fact that it tries to import stuff from the jwst package, but the stcal test environment is setup to only test "common" code that's observatory-agnostic and hence doesn't include the jwst package in the build. If you want to use jwst specifics in the test, then the test module will need to be moved to /jwst/ramp_fit/tests/, i.e. in the jwst pkg instead of stcal. You'll need to create a separate PR against jwst for that. |
|
Howard,
No the fix is in ols_fit.py. It’s the three lines of code starting at 1662.
I’m confused. How can STCAL process models that are defined in JWST without importing JWST?
Mike
From: Howard Bushouse ***@***.***>
Reply-To: spacetelescope/stcal ***@***.***>
Date: Monday, April 25, 2022 at 2:28 PM
To: spacetelescope/stcal ***@***.***>
Cc: Michael Regan ***@***.***>, Author ***@***.***>
Subject: Re: [spacetelescope/stcal] Fix for jumps in first good group after dropping groups (PR #84)
So the problems with the MIRI ramp fitting results were actually in the jump step, as opposed to ramp_fit?
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/spacetelescope/stcal/pull/84*issuecomment-1108901337__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!17SnHISSHX2fsCfBi3KrfVQzb11jNUxFUtx9d-h5F8NgIh2SW7fDrHqze1XCAT-CAN9BEsIYZOUjW42eOB5x_LU$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AF6KJLUYB4JD4YQ2JJ5WB3DVG3P3FANCNFSM5UFBY32Q__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!17SnHISSHX2fsCfBi3KrfVQzb11jNUxFUtx9d-h5F8NgIh2SW7fDrHqze1XCAT-CAN9BEsIYZOUjW42eQrJApMo$>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
Yes, I realized my mistake about the jump vs ramp_fit steps and retracted the question. Regarding stcal vs jwst, by the time data gets down to these routines in the stcal package it's all in the form of generic numpy arrays and such, all completely detached from any dependencies on jwst-specific datamodels. Notice that none of the modules in stcal/ramp_fitting import the jwst package. Anything that's jwst-specific is in the jwst package and rebundled by the top-level step module to just send generic data structures to the routines in stcal. That's in order to allow sharing by other pipeline (e.g. Roman) without having to import the jwst package. |
|
Howard,
This seems kind of dangerous. STCAL will be making assumptions about the content of the incoming arrays. The two packages are linked but with no way to force the dependancy. Isn’t it better to define the models in STCAL and have JWST and ROMAN import from STCAL?
Mike
From: Howard Bushouse ***@***.***>
Reply-To: spacetelescope/stcal ***@***.***>
Date: Monday, April 25, 2022 at 3:02 PM
To: spacetelescope/stcal ***@***.***>
Cc: Michael Regan ***@***.***>, Mention ***@***.***>
Subject: Re: [spacetelescope/stcal] Fix for jumps in first good group after dropping groups (PR #84)
Yes, I realized my mistake about the jump vs ramp_fit steps and retracted the question.
Regarding stcal vs jwst, by the time data gets down to these routines in the stcal package it's all in the form of generic numpy arrays and such, all completely detached from any dependencies on jwst-specific datamodels. Notice that none of the modules in stcal/ramp_fitting import the jwst package. Anything that's jwst-specific is in the jwst package and rebundled by the top-level step module to just send generic data structures to the routines in stcal. That's in order to allow sharing by other pipeline (e.g. Roman) without having to import the jwst package.
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/spacetelescope/stcal/pull/84*issuecomment-1108929895__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!zhZK873lhDVW2zpE-Mu-CSfIUUPUqm2K1nSDgXoEinNhGzQPfmqMgjij8HOfDuXHUeM5uwIM56E1JVQvh1Nli_s$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AF6KJLRXXBBKODHHOAB3T6LVG3T27ANCNFSM5UFBY32Q__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!zhZK873lhDVW2zpE-Mu-CSfIUUPUqm2K1nSDgXoEinNhGzQPfmqMgjij8HOfDuXHUeM5uwIM56E1JVQvyfF3GS0$>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
To avoid the dependency on the JWST package. I deleted the unit test.
|
@mwregan2 reports that this fix has been used fairly extensively by the MIRI team to reprocess their data and it correctly solves the previous problems. So this is ready to merge (which I'll do). |
Resolves JP-2572
Resolves JP-2574
It has been tested against MIRI imager and MRS multi-integration exposures where it correctly ignored the first good groups when there was a jump in the 2nd good group and the early groups were set to DO NOT USE.