spacetelescope / stdatamodels

https://stdatamodels.readthedocs.io
Other
5 stars 25 forks source link

add fits keywords to NRMModel schema #361

Closed braingram closed 1 week ago

braingram commented 2 weeks ago

Closes #359

The NRM reference files contain FITS keywords useful to CAL. https://jwst-crds.stsci.edu/browse/jwst_niriss_nrm_0001.fits

This PR adds those keywords to the schema so they are accessible via the datamodel interface:

Regression tests running at https://github.com/spacetelescope/RegressionTests/actions/runs/11802888443

Tasks

news fragment change types... - ``changes/.feature.rst``: new feature - ``changes/.bugfix.rst``: fixes an issue - ``changes/.doc.rst``: documentation change - ``changes/.removal.rst``: deprecation or removal of public API - ``changes/.misc.rst``: infrastructure or miscellaneous change
codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 67.52%. Comparing base (1e16207) to head (84143a1). Report is 3 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #361 +/- ## ======================================= Coverage 67.52% 67.52% ======================================= Files 114 114 Lines 5916 5916 ======================================= Hits 3995 3995 Misses 1921 1921 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

rcooper295 commented 2 weeks ago

I think this looks good! To make sure I understand, these will not be accessed via the meta tree but rather from the "top level" of the datamodel? I.e. model.x_a1 rather than model.meta.x_a1?

braingram commented 2 weeks ago

I think this looks good! To make sure I understand, these will not be accessed via the meta tree but rather from the "top level" of the datamodel? I.e. model.x_a1 rather than model.meta.x_a1?

Exactly! Feel free to give it a try with the current NRM reference file (and this branch) if you're curious.

rcooper295 commented 2 weeks ago

Thanks! I was able to try it with your branch and it looks good! I realized that there may be two additional keywords whose values are currently hardcoded but that we could read in from this model instead, but I need to confirm if they are actually the same as values we already have in the FITS header, and if so, what we should call the attributes. I should have an answer on that tomorrow. (For more details, this is the message I sent the AMI team):

mask_definitions.py contains:

self.activeD = 6.559 * m  # webbpsf kwd DIAM  - not a 'circle including all holes'
self.OD = 6.610645669291339 * m  # Full pupil file size, incl padding, webbpsf kwd PUPLDIAM

while the NRM reference file currently contains:

DIAM    =             6.559348 / Flat-to-flat distance across pupil in V3 axis  
PUPLDIAM=             6.603464 / Full pupil file size, incl padding.            
PUPL_CRC=             6.603464 / Circumscribing diameter for JWST primary

I'm not sure if activeD should be the same as DIAM (it is, at least up to the fourth decimal place) and if OD should be the same as PUPLDIAM (it isn't quite) but if these are referring to the same quantity we should probably make them consistent.

braingram commented 2 weeks ago

I'll mark this as draft until we hear back about the extra keywords.

rcooper295 commented 2 weeks ago

I spoke to my AMI team members and chased down the source of these numbers (or what we should be using) and we'd like to add these two:

 diameter:
      fits_keyword: DIAM
      title: "[m] Flat-to-flat distance across pupil in V3 axis"
pupil_circumscribed:
      fits_keyword: PUPL_CRC
      title: "[m] Circumscribing diameter for JWST primary"

Thanks for your patience!!