Skip to content

Fix outlier blot filenames #7784

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.

Sign up for GitHub

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

Jump to bottom
Merged
hbushouse merged 1 commit into from
Aug 2, 2023
Merged

Fix outlier blot filenames #7784

hbushouse merged 1 commit into from
Aug 2, 2023

Conversation

@jdavies-st
Copy link
Collaborator

@jdavies-st jdavies-st commented Jul 31, 2023 •
edited
Loading

This PR addresses very long, redundant file names in outlier_detection intermediate products which appears to be a bug in the call to Step.make_output_path() with the arg basename is used instead of basepath. Currently, blot produces files from standard science associations by default with names like:

jw02234-o010_20230627t013403_image3_00001_asn_o010_jw02234010001_02101_00001_nrcalong_blot.fits
jw02234-o010_20230627t013403_image3_00001_asn_o010_jw02234010001_02101_00001_nrcblong_blot.fits
jw02234-o010_20230627t013403_image3_00001_asn_o010_jw02234010001_02101_00002_nrcalong_blot.fits
jw02234-o010_20230627t013403_image3_00001_asn_o010_jw02234010001_02101_00002_nrcblong_blot.fits
jw02234-o010_20230627t013403_image3_00001_asn_o010_jw02234010001_02101_00003_nrcalong_blot.fits
jw02234-o010_20230627t013403_image3_00001_asn_o010_jw02234010001_02101_00003_nrcblong_blot.fits
jw02234-o010_20230627t013403_image3_00001_asn_o010_jw02234010001_02101_00004_nrcalong_blot.fits
jw02234-o010_20230627t013403_image3_00001_asn_o010_jw02234010001_02101_00004_nrcblong_blot.fits

Not ideal. Since the blot image in outlier_detection is based on an exposure as modified by the rest of the exposures in the association, it should follow the same naming scheme as the _crf step results, which are:

jw02234010001_02101_00001_nrcalong_o010_crf.fits
jw02234010001_02101_00001_nrcblong_o010_crf.fits
jw02234010001_02101_00002_nrcalong_o010_crf.fits
jw02234010001_02101_00002_nrcblong_o010_crf.fits
jw02234010001_02101_00003_nrcalong_o010_crf.fits
jw02234010001_02101_00003_nrcblong_o010_crf.fits
jw02234010001_02101_00004_nrcalong_o010_crf.fits
jw02234010001_02101_00004_nrcblong_o010_crf.fits

This PR changes the default naming of blot intermediate products to be:

jw02234010001_02101_00001_nrcalong_o010_blot.fits
jw02234010001_02101_00001_nrcblong_o010_blot.fits
jw02234010001_02101_00002_nrcalong_o010_blot.fits
jw02234010001_02101_00002_nrcblong_o010_blot.fits
jw02234010001_02101_00003_nrcalong_o010_blot.fits
jw02234010001_02101_00003_nrcblong_o010_blot.fits
jw02234010001_02101_00004_nrcalong_o010_blot.fits
jw02234010001_02101_00004_nrcblong_o010_blot.fits

This PR also fixes the logging in resample to be consistent with what Step.save_model() echos to the log when writing out a file, making the log easier to read. It also records in the log the intermediate products that are now written out by default in the step.

So it goes from this:

2023-07-31 12:21:37,428 - stpipe.Image3Pipeline.outlier_detection - INFO - 2 exposures to drizzle together
2023-07-31 12:21:41,044 - stpipe.Image3Pipeline.outlier_detection - INFO - Drizzling (2048, 2048) --> (2206, 4959)
2023-07-31 12:21:45,872 - stpipe.Image3Pipeline.outlier_detection - INFO - Drizzling (2048, 2048) --> (2206, 4959)
2023-07-31 12:21:47,221 - stpipe.Image3Pipeline.outlier_detection - INFO - Exposure jw02234010001_02101_00001_nrcblong_outlier_i2d.fits saved to file
2023-07-31 12:22:25,246 - stpipe.Image3Pipeline.outlier_detection - INFO - Blotting median...
2023-07-31 12:22:28,320 - stpipe.Image3Pipeline.outlier_detection - INFO - Blotting (2048, 2048) <-- (2206, 4959)
2023-07-31 12:22:31,561 - stpipe.Image3Pipeline.outlier_detection - INFO - Blotting (2048, 2048) <-- (2206, 4959)

to this:

2023-07-31 13:45:56,211 - stpipe.Image3Pipeline.outlier_detection - INFO - 2 exposures to drizzle together
2023-07-31 13:45:59,879 - stpipe.Image3Pipeline.outlier_detection - INFO - Drizzling (2048, 2048) --> (2206, 4959)
2023-07-31 13:46:04,708 - stpipe.Image3Pipeline.outlier_detection - INFO - Drizzling (2048, 2048) --> (2206, 4959)
2023-07-31 13:46:06,195 - stpipe.Image3Pipeline.outlier_detection - INFO - Saved model in jw02234010001_02101_00001_nrcblong_outlier_i2d.fits
2023-07-31 13:46:06,423 - stpipe.Image3Pipeline.outlier_detection - INFO - Computing median
2023-07-31 13:46:43,810 - stpipe.Image3Pipeline.outlier_detection - INFO - Saved model in jw02234010001_02101_00002_nrcalong_outlier_o010_median.fits
2023-07-31 13:46:43,819 - stpipe.Image3Pipeline.outlier_detection - INFO - Blotting median
2023-07-31 13:46:47,008 - stpipe.Image3Pipeline.outlier_detection - INFO - Blotting (2048, 2048) <-- (2206, 4959)
2023-07-31 13:46:47,401 - stpipe.Image3Pipeline.outlier_detection - INFO - Saved model in jw02234010001_02101_00002_nrcalong_o010_blot.fits
2023-07-31 13:46:50,467 - stpipe.Image3Pipeline.outlier_detection - INFO - Blotting (2048, 2048) <-- (2206, 4959)
2023-07-31 13:46:50,782 - stpipe.Image3Pipeline.outlier_detection - INFO - Saved model in jw02234010001_02101_00002_nrcblong_o010_blot.fits

I updated the docs to indicate what these intermediate products are. I also note that save_intermediate_results no longer controls whether these products are written out, as they are always written out now. I'm not doing anything about that right now, only updating the docs to indicate what the current pipeline step does.

Checklist for maintainers

  • added entry in CHANGES.rst within the relevant release section
  • updated or added relevant tests
  • updated relevant documentation
  • added relevant milestone
  • added relevant label(s)
  • ran regression tests, post a link to the Jenkins job below.
    How to run regression tests on a PR
  • Make sure the JIRA ticket is resolved properly

@jdavies-st jdavies-st requested a review from a team as a code owner July 31, 2023 12:27
@hbushouse hbushouse added this to the Build 10.0 milestone Jul 31, 2023
hbushouse
hbushouse reviewed Jul 31, 2023
View reviewed changes
Copy link
Collaborator

@hbushouse hbushouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updates look good. Needs a change log entry. And a regtest run wouldn't hurt (let me know if you're not allowed to run those anymore).

@codecov
Copy link

codecov bot commented Jul 31, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01% ⚠️

Comparison is base (e356cae) 76.59% compared to head (93df344) 76.58%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #7784      +/-   ##
==========================================
- Coverage   76.59%   76.58%   -0.01%     
==========================================
  Files         456      456              
  Lines       36963    36960       -3     
==========================================
- Hits        28310    28307       -3     
  Misses       8653     8653
Flag Coverage Δ *Carryforward flag
nightly 77.41% <ø> (-0.01%) ⬇️ Carriedforward from e356cae

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

Files Changed Coverage Δ
jwst/outlier_detection/outlier_detection.py 89.76% <100.00%> (-0.15%) ⬇️
jwst/resample/resample.py 95.95% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jdavies-st
Copy link
Collaborator Author

jdavies-st commented Jul 31, 2023

I can't run the regtests anymore, but I've updated the change log. 👍

@jdavies-st
Copy link
Collaborator Author

jdavies-st commented Aug 1, 2023 •
edited
Loading

Updated changelog is on the branch, but for some reason the new commit is not showing up in this PR.

I squashed the commits and force-pushed and now it finds it.

@jdavies-st jdavies-st force-pushed the outlier-detection-filename-fix branch from 96a6997 to 93df344 Compare August 1, 2023 11:34
hbushouse
hbushouse approved these changes Aug 1, 2023
View reviewed changes
Copy link
Collaborator

@hbushouse hbushouse 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. I'll kick off a regtest run.

@hbushouse
Copy link
Collaborator

hbushouse commented Aug 1, 2023

Regtest started at https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/828

@hbushouse hbushouse merged commit 12030b5 into spacetelescope:master Aug 2, 2023
@jdavies-st jdavies-st deleted the outlier-detection-filename-fix branch August 2, 2023 08:43
mairanteodoro pushed a commit to mairanteodoro/jwst that referenced this pull request Sep 20, 2023
@jdavies-st @mairanteodoro
Fix outlier blot filenames (spacetelescope#7784)
c96f2e7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@hbushouse hbushouse hbushouse approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

documentation outlier_detection resample

Projects

None yet

Milestone

Build 10.0

Development

Successfully merging this pull request may close these issues.

2 participants

@jdavies-st @hbushouse