Skip to content
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

Update and fix NIRSpec MOS master bkg regtests #5891

Merged
merged 1 commit into from
Mar 18, 2021

Conversation

hbushouse
Copy link
Collaborator

@hbushouse hbushouse commented Mar 18, 2021

Description

This PR addresses a problem I recently noticed in the master bkg regression tests for the NIRSpec MOS mode. First, the existing test that used a user-supplied master bkg spectrum was still running the calspec3 master bkg step, instead of using the new NIRSpec-slit-specific version of the master bkg step that gets called from calspec2. I removed that entire test and replaced it with one that runs the data through calspec2, along with a user-supplied mbkg spectrum. I also fixed the naming and location of truth files for the equivalent test that runs MOS data through calspec2 and computes the mbkg spectrum from available bkg slits in the exposure.

All necessary input files and truth files for the new test have been uploaded to artifactory and the location of truth files for the non-user-supplied case has been moved.

Related to JP-1935

Checklist

  • Tests
  • Documentation (N/A: no docs for regtests)
  • Change log (N/A: not required for regtests)
  • Milestone
  • Label(s)

@codecov
Copy link

codecov bot commented Mar 18, 2021

Codecov Report

Merging #5891 (a27959c) into master (9759493) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5891      +/-   ##
==========================================
- Coverage   77.84%   77.83%   -0.01%     
==========================================
  Files         401      401              
  Lines       35275    35285      +10     
==========================================
+ Hits        27459    27465       +6     
- Misses       7816     7820       +4     
Flag Coverage Δ *Carryforward flag
nightly 77.84% <ø> (ø) Carriedforward from 9759493
unit 56.04% <ø> (?)

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

Impacted Files Coverage Δ
jwst/associations/asn_edit.py 66.66% <0.00%> (-0.75%) ⬇️
jwst/associations/lib/process_list.py 73.49% <0.00%> (-0.59%) ⬇️
jwst/fits_generator/template.py 16.98% <0.00%> (-0.27%) ⬇️
jwst/timeconversion/time_conversion.py 10.96% <0.00%> (-0.08%) ⬇️
jwst/datamodels/make_header.py 91.38% <0.00%> (+0.02%) ⬆️
jwst/associations/lib/product_utils.py 94.82% <0.00%> (+0.09%) ⬆️
jwst/regtest/conftest.py 67.46% <0.00%> (+1.20%) ⬆️
jwst/tso_photometry/tso_photometry_step.py 77.41% <0.00%> (+1.61%) ⬆️

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 9759493...a27959c. Read the comment docs.

@hbushouse hbushouse requested a review from tapastro March 18, 2021 13:48
Copy link
Contributor

@tapastro tapastro left a 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, but a question: it looks as though the default level 3 master background step has a .cfg file you could use with strun for non-nirspec-slit observations, but it looks like here you use the calwebb_spec2 input to pull the nrs_slits-specific step. Is this the only easy way to interact with this step? I'm still getting my head around strun calls without cfg input, so maybe there's another way.

@hbushouse
Copy link
Collaborator Author

Looks good to me, but a question: it looks as though the default level 3 master background step has a .cfg file you could use with strun for non-nirspec-slit observations, but it looks like here you use the calwebb_spec2 input to pull the nrs_slits-specific step. Is this the only easy way to interact with this step? I'm still getting my head around strun calls without cfg input, so maybe there's another way.

Theoretically I believe you could test only the master bkg subtraction by just calling the nirspec-slits version of the master bkg step, but in order for it to work right (i.e. give the right scientific answer) it has to be called mid-stream within the calwebb_spec2 processing flow (e.g. after the exposure has been processed up through the extract_2d and srctype steps) and then the resulting output of the mbkg step gets reinjected into the remaining flow of calwebb_spec2. So in order to just call the nirspec-slits version of mbkg by itself, the input data would need to have that partial calwebb_spec2 processing applied to them. I figured it was easy and more thorough to just run the entire calwebb_spec2 pipeline on a given exposure, let it do all the complicated data flow handling, and then test the final calwebb_spec2 output products (rather than just testing the result of mbkg by itself). That way we also make sure that changes in calwebb_spec2 itself don't have any unintended side effects on the mbkg process.

@tapastro
Copy link
Contributor

Theoretically I believe you could test only the master bkg subtraction by just calling the nirspec-slits version of the master bkg step, but in order for it to work right (i.e. give the right scientific answer) it has to be called mid-stream within the calwebb_spec2 processing flow (e.g. after the exposure has been processed up through the extract_2d and srctype steps) and then the resulting output of the mbkg step gets reinjected into the remaining flow of calwebb_spec2. So in order to just call the nirspec-slits version of mbkg by itself, the input data would need to have that partial calwebb_spec2 processing applied to them. I figured it was easy and more thorough to just run the entire calwebb_spec2 pipeline on a given exposure, let it do all the complicated data flow handling, and then test the final calwebb_spec2 output products (rather than just testing the result of mbkg by itself). That way we also make sure that changes in calwebb_spec2 itself don't have any unintended side effects on the mbkg process.

That makes sense to me. PR review and a lesson, what a deal!

@hbushouse hbushouse merged commit dbe28ff into spacetelescope:master Mar 18, 2021
@hbushouse hbushouse deleted the nrs_regtest branch March 18, 2021 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants