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

Speed up NRS IFU footprint computation #5969

Merged
merged 6 commits into from
Apr 21, 2021

Conversation

nden
Copy link
Collaborator

@nden nden commented Apr 19, 2021

Description

This PR speeds up the computation of NIRSpec IFU footprints by using the WCS transforms directly,
instead of through the WCS object. Most of the transforms are reused and only slice specific differences are
computed.

The computed spectral range is saved in meta.wcsinfo.spectral_range. It's not saved in the FITS file.
This is necessary for improving cube_build performance.

The "true" bounding box in the test was computed using the previous implementation with evaluating the WCS object.

Checklist

  • Tests
  • Documentation
  • Change log
  • Milestone
  • Label(s)

slit_name : int or str
Slit.name of an open slit.
wavelength_range: list
Wavelength range for the combination of fliter and grating.
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: fliter -> filter

(1874.8184744639657, 1929.9072657798927))}

hdul = create_nirspec_ifu_file("F290LP", "G140M")
im = datamodels.ImageModel(hdul)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be an IFUImageModel or does it not matter to the usage here?

@codecov
Copy link

codecov bot commented Apr 19, 2021

Codecov Report

Merging #5969 (d724473) into master (a1bac0e) will decrease coverage by 0.03%.
The diff coverage is 72.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5969      +/-   ##
==========================================
- Coverage   77.81%   77.78%   -0.04%     
==========================================
  Files         404      404              
  Lines       35352    35464     +112     
==========================================
+ Hits        27509    27584      +75     
- Misses       7843     7880      +37     
Flag Coverage Δ *Carryforward flag
nightly 56.15% <73.68%> (ø) Carriedforward from 0eae9d1
unit 56.15% <53.22%> (-21.66%) ⬇️

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

Impacted Files Coverage Δ
jwst/assign_wcs/util.py 75.56% <54.90%> (-4.19%) ⬇️
jwst/assign_wcs/nirspec.py 92.75% <100.00%> (-1.21%) ⬇️
jwst/refpix/reference_pixels.py 92.52% <0.00%> (+0.86%) ⬆️

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 a1bac0e...d724473. Read the comment docs.

-------
footprint : ndarray
The spatial footprint
spctral_region : tuple
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be spectral_region (not spectral_region)

footprint : ndarray
The spatial footprint
spctral_region : tuple
The wavwlength range for the observation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo - should be wavelength

Copy link
Collaborator

@jemorrison jemorrison left a comment

Choose a reason for hiding this comment

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

Except for a few typos it looks fine to me.

@jemorrison
Copy link
Collaborator

Can we merge this so I can use it in cube_build.

@nden nden merged commit 63b5caa into spacetelescope:master Apr 21, 2021
@nden nden deleted the speed-nrs-ifu-footprint branch July 26, 2023 19:27
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.

3 participants