spacetelescope / jwst

Python library for science observations from the James Webb Space Telescope
https://jwst-pipeline.readthedocs.io/en/latest/
Other
569 stars 167 forks source link

Make use of XSCI_REF/YSCI_REF more robust against missing keywords #4239

Closed stscijgbot closed 4 years ago

stscijgbot commented 5 years ago

Issue JP-1129 was created by Howard Bushouse:

There have been a number of occasions where one of the calwebb_tso3 steps crashes due to the XSCI_REF/YSCI_REF (or CRPIX1/CRPIX2) keywords not being present in the input header (due to upstream problems with WCS construction). The steps that make use of these keywords should have try/except blocks added so that more information error messages are emitted when these values aren't available.

stscijgbot commented 4 years ago

Comment by Howard Bushouse: It appears that the only pipeline modules that make use of the XREF_SCI and YREF_SCI keywords are:

  assign_wcs/nircam.py

  extract_2d/grisms.py

The only similar use of CRPIX1 and CRPIX2 for TSO object positions occurs in:

  tso_photometry/tso_photometry_step.py

stscijgbot commented 4 years ago

Comment by Philip Hodge: siaf_xref_sci, etc., are in wcsinfo.schema.yaml, so the attributes ought to exist regardless of whether the keywords are present:

    m = datamodels.ImageModel()     m.meta.wcsinfo.siaf_xref_sci is None     True

Should the test be a check for None, rather than using try/except?

stscijgbot commented 4 years ago

Comment by Howard Bushouse: Yes, a test for None before attempting an operation on them should work too.

stscijgbot commented 4 years ago

Comment by James White: What's the desired outcome when those keys are not present (i.e. None)?

 

For example in assign_wcs/nircam.tsgrism if those keywords are None should an error be raised? Or should the final pipeline list just have None in place of those variables that used those keywords? (my guess would be option 2)

stscijgbot commented 4 years ago

Comment by James White: Then perhaps add a warning that those items are being omitted?

stscijgbot commented 4 years ago

Comment by Howard Bushouse: These values (crpix1/2 for ts-imaging and xref_sci/yref_sci for ts-grism) are absolutely necessary for calibration at level-2b (calwebb_xxxx2) and level-3 (calwebb_xxxx3), so their absence should result in an error and have the pipeline shutdown. There's already logic in at least one of the level-2b pipelines to check whether assign_wcs was skipped (due to errors like missing keywords) and shutdown the rest of the pipeline.

stscijgbot commented 4 years ago

Comment by James White: Ok, great. Thanks [~bushouse]