AL-479: Preparing ramp fit code for removal from JWST and moved to STCAL #6010

kmacdonald-stsci · 2021-05-05T18:43:32Z

Closes #

Resolves JP-

Description

This PR addresses preparing JWST ramp fit to be moved to STCAL. JWST dependencies have been almost entirely eliminated. In particular, data models are no longer returned from ramp_fit. Tuples of arrays that can easily be used to create data models in step code are now used. The only JWST dependencies that still exist are DQ flags. I have structured the code so that this dependency can easily be removed. The only other dependency that will need to change are path dependencies. I have included path dependency changes in this code as comments.

GLS and multiprocessing code have been commented out as not needed for the moment.

With these changes, all ramp fitting unit tests pass, as well as all regression tests, except for a known failure related to jwst/regtest/test_nirspec_verify.py::test_verify_detector1[rate]. A small number of pixels fail, but they are the same failures that currently fail in master.

Checklist

…ting.

…o the step code tests.

…than data models. Changed the standard ramp fit tests to match this input.

… simple numpy arrays. Removed reference to jwst.reffile_utils from ramp fit processing code and moved it to ramp fit step code.

…p code.

…ple of arrays. All tests and regression tests have passed.

…to the fitting of each integration.

…l references to datamodels can now be removed. All tests, including regression tests, pass.

codecov · 2021-05-05T19:23:07Z

Codecov Report

Merging #6010 (5e97782) into master (14749b0) will decrease coverage by 0.32%.
The diff coverage is 90.28%.

@@            Coverage Diff             @@
##           master    #6010      +/-   ##
==========================================
- Coverage   77.91%   77.58%   -0.33%     
==========================================
  Files         404      404              
  Lines       35418    37227    +1809     
==========================================
+ Hits        27595    28882    +1287     
- Misses       7823     8345     +522

Flag	Coverage Δ		*Carryforward flag
nightly	`77.91% <86.20%> (ø)`		Carriedforward from 2202337
unit	`57.04% <92.00%> (+0.75%)`	⬆️

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files	Coverage Δ
jwst/ramp_fitting/ols_fit.py	`89.71% <78.94%> (+5.67%)`	⬆️
jwst/ramp_fitting/ramp_fit_step.py	`90.59% <90.69%> (+0.12%)`	⬆️
jwst/ramp_fitting/utils.py	`92.40% <97.43%> (-1.85%)`	⬇️
jwst/ramp_fitting/gls_fit.py	`59.68% <100.00%> (ø)`
jwst/ramp_fitting/ramp_fit.py	`100.00% <100.00%> (ø)`
jwst/pipeline/calwebb_spec2.py	`77.61% <0.00%> (-18.43%)`	⬇️
jwst/cube_build/ifu_cube.py	`57.31% <0.00%> (-14.22%)`	⬇️
jwst/cube_build/cube_build_wcs_util.py	`68.78% <0.00%> (-6.22%)`	⬇️
jwst/cube_build/cube_build_step.py	`63.56% <0.00%> (-2.01%)`	⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14749b0...5e97782. Read the comment docs.

dmggh

Looks good so far. Yes, I see that, as you wrote in the description, you have modified 'almost' all of the JWST code (that will be moved to STCAL-ols_fit.py, ramp_fit.py, utils.py ) involving data models to instead use tuples. Some of the functions within these modules that call other functions within these modules still use JWST data models - which I think is what you meant by 'almost'. These data models will also be replaced by tuples, right ?

kmacdonald-stsci · 2021-05-06T14:57:28Z

The model parameter in ramp_fit is assumed to be a data model, specifically a JWST RampModel like data model. It is expected to have certain features, like the data, dq, etc. arrays, as well as the meta object. These variables and variable names are expected to be the same across pipelines.

nden · 2021-05-10T13:13:55Z

@dmggh @kmacdonald-stsci Does this need more work or is it ready for final review?

kmacdonald-stsci · 2021-05-10T13:15:09Z

This is ready for final review.

dmggh

Aside from the datamodels code you have noted, it looks like the ramp_fit code is ready to be moved from JWST to STCAL.

jdavies-st

Looks good. Just some minor comments below. 👍

jwst/ramp_fitting/ramp_fit_step.py

jwst/ramp_fitting/tests/test_ramp_fit_cases.py

jdavies-st · 2021-05-10T20:01:31Z

jwst/ramp_fitting/tests/test_ramp_fit_step.py

+@pytest.fixture(scope="module")
+def generate_miri_reffiles(tmpdir_factory):
+    gainfile = str(tmpdir_factory.mktemp("data").join("gain.fits"))
+    readnoisefile = str(tmpdir_factory.mktemp("data").join('readnoise.fits'))
+


Is there a reason to have tmpdir_factory here and have it scoped to module? This fixture doesn't write anything out. The mock reffiles are just returned in memory.

The noise models are getting written to the temp directory. Only the file names are being returned, not the reference to the models in memory.

jwst/ramp_fitting/tests/test_ramp_fit_step.py

jwst/ramp_fitting/utils.py

jwst/ramp_fitting/ramp_fit_step.py

… file names through fixture.

jdavies-st · 2021-05-11T18:34:52Z

This looks good. Ready to merge. Thanks!

…ed to STCAL (spacetelescope#6010)" This reverts commit 5a9aaf8.

* Revert "AL-479: Move ramp fit code from JWST to STCAL. (#6023)" This reverts commit 85feb11. * Revert "AL-479: Preparing ramp fit code for removal from JWST and moved to STCAL (#6010)" This reverts commit 5a9aaf8. * Turn on ramp_fit multiprocessing tests * Update CHANGES.rst

kmacdonald-stsci added 20 commits May 4, 2021 08:33

Adding step test.

720d04d

Removed subarray test from ramp fit testing to ramp fit step code tes…

918eb8a

…ting.

Changing read noise variable names to remove reference to model.

8ab5c9f

Style changes to ramp fit tests.

560449b

Removing unneeded function now that subarray testing has been moved t…

ffb92b5

…o the step code tests.

Changed ramp_fit input to take arrays for gain and readnoise, rather …

c2fd44c

…than data models. Changed the standard ramp fit tests to match this input.

Changing input parameters for read noise and gain from data models to…

592b447

… simple numpy arrays. Removed reference to jwst.reffile_utils from ramp fit processing code and moved it to ramp fit step code.

Moving data units assignment out of ramp fitting code to ramp fit ste…

483a8dd

…p code.

Removing unnecessary comments.

ad54a78

Ready for integration time removal from module into step code.

51369d0

Moving integration time calculation from the ramp module to step code.

b7472f2

Preparing variable names for returning tuples, instead of models.

d4af90f

Prepping variable names in tests for returning arrays.

f1e2894

Removing GLS code by commenting out the entire file.

cf10143

Changing output model to a tuple called 'image_info'.

b621208

Changed the return value from ramp_fit from a JWST ImageModel to a tu…

51b5438

…ple of arrays. All tests and regression tests have passed.

Adding progress comments.

49b6258

Changed CubeModel return value to a tuple of computed arrays related …

c566a4c

…to the fitting of each integration.

Changed all outputs to be tuples of arrays, instead of datamodels. Al…

9bc6ff0

…l references to datamodels can now be removed. All tests, including regression tests, pass.

Cleaned up code and readied code to be moved from JWST to STCAL.

a3c894c

kmacdonald-stsci requested review from dmggh, hbushouse and jdavies-st May 5, 2021 18:43

kmacdonald-stsci changed the title ~~AL-479~~ AL-479: Preparing ramp fit code for removal from JWST and moved to STCAL May 5, 2021

Adding to change log.

6054d34

dmggh reviewed May 6, 2021

View reviewed changes

dmggh approved these changes May 10, 2021

View reviewed changes

jdavies-st approved these changes May 10, 2021

View reviewed changes

kmacdonald-stsci added 2 commits May 10, 2021 18:06

Made changes based on code review.

d75bc7c

Making style changes (removed extra line).

2202337

jdavies-st added this to the Build 7.8 milestone May 11, 2021

jdavies-st added the ramp_fitting label May 11, 2021

jdavies-st reviewed May 11, 2021

View reviewed changes

jwst/ramp_fitting/ramp_fit_step.py Outdated Show resolved Hide resolved

jdavies-st reviewed May 11, 2021

View reviewed changes

jwst/ramp_fitting/ramp_fit_step.py Outdated Show resolved Hide resolved

kmacdonald-stsci added 2 commits May 11, 2021 10:32

Making changes based on code review.

ed3b2f3

Removing write to disk for a test and passing back models, instead of…

5e97782

… file names through fixture.

jdavies-st merged commit 5a9aaf8 into spacetelescope:master May 11, 2021

kmacdonald-stsci deleted the jwst_rem_dep branch May 12, 2021 12:33

jdavies-st added a commit to jdavies-st/jwst that referenced this pull request Jun 4, 2021

Revert "AL-479: Preparing ramp fit code for removal from JWST and mov…

4e2462b

…ed to STCAL (spacetelescope#6010)" This reverts commit 5a9aaf8.

jdavies-st added a commit to jdavies-st/jwst that referenced this pull request Jun 7, 2021

Revert "AL-479: Preparing ramp fit code for removal from JWST and mov…

631a884

…ed to STCAL (spacetelescope#6010)" This reverts commit 5a9aaf8.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AL-479: Preparing ramp fit code for removal from JWST and moved to STCAL #6010

AL-479: Preparing ramp fit code for removal from JWST and moved to STCAL #6010

kmacdonald-stsci commented May 5, 2021 •

edited by jdavies-st

Loading

codecov bot commented May 5, 2021 •

edited

Loading

dmggh left a comment

kmacdonald-stsci commented May 6, 2021

nden commented May 10, 2021

kmacdonald-stsci commented May 10, 2021

dmggh left a comment

jdavies-st left a comment

jdavies-st May 10, 2021

kmacdonald-stsci May 10, 2021

jdavies-st commented May 11, 2021

AL-479: Preparing ramp fit code for removal from JWST and moved to STCAL #6010

AL-479: Preparing ramp fit code for removal from JWST and moved to STCAL #6010

Conversation

kmacdonald-stsci commented May 5, 2021 • edited by jdavies-st Loading

codecov bot commented May 5, 2021 • edited Loading

Codecov Report

dmggh left a comment

Choose a reason for hiding this comment

kmacdonald-stsci commented May 6, 2021

nden commented May 10, 2021

kmacdonald-stsci commented May 10, 2021

dmggh left a comment

Choose a reason for hiding this comment

jdavies-st left a comment

Choose a reason for hiding this comment

jdavies-st May 10, 2021

Choose a reason for hiding this comment

kmacdonald-stsci May 10, 2021

Choose a reason for hiding this comment

jdavies-st commented May 11, 2021

kmacdonald-stsci commented May 5, 2021 •

edited by jdavies-st

Loading

codecov bot commented May 5, 2021 •

edited

Loading