JP-2922: Use new source_id syntax for level-3 products #8442
Conversation
github-actions
bot
added
assign_wcs
associations
pipeline
NIRSPEC
spec3 pipeline
labels
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8442 +/- ##
==========================================
- Coverage 57.97% 57.89% -0.09%
==========================================
Files 387 387
Lines 38830 38849 +19
==========================================
- Hits 22513 22491 -22
- Misses 16317 16358 +41 ☔ View full report in Codecov by Sentry. |
|
Latest regression test run is here: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1453/ Unfortunately it includes a large number of unrelated failures, due to an update to a NIRCam photom ref file earlier today. Remaining related failures are due to expected differences in file names and source_id values in multislit files. Will make another run once the truth files have been updated for the NIRCam photom change, just to make it simpler to sort out. |
jwst/assign_wcs/nirspec.py
Outdated
| source_alias = "vrt_{}".format(abs(source_id)) | ||
| stellarity = 0.0 | ||
| source_ra = 0.0 | ||
| source_dec = 0.0 |
There was a problem hiding this comment.
This is a change in behavior. Previously source_x/ypos and source_ra/dec for virtual slits came from the msa file. I think it makes sense to get them from the msa file because the assumption is there are sources in those slits. For a program I checked those are set in the msa file and this code resets them to 0 which we probably don't want to do.
In terms of setting these attributes should virtual slits be treated like source slits?
There was a problem hiding this comment.
Wow, you're right! I was (blindly) assuming that slits with a virtual source, identified by their negative source_id value in the MSA shutter table, would not have any corresponding info in the MSA source table, and hence was not even bothering to load it. But they do have entries in the source table. For example, here are the shutter table rows for virtual slit 87, which has source_id=-70. And there is a corresponding row in the source table for source_id=-70:
FITS_rec([(87, 88, 4, 345, 1, -70, 'N', 'OPEN', 0.5, 0.5, 1, 'Y'),
(87, 88, 4, 345, 1, 0, 'Y', 'OPEN', nan, nan, 2, 'N'),
(87, 88, 4, 345, 1, 0, 'Y', 'OPEN', nan, nan, 3, 'N')],
(1345, -70, '1345_-70', '-09:-039:0.0000 +53:00:57.29', 215.1912160802715, 53.01591471620116, 'None', 0.0)
The x/ypos values are set to 0.5 in the shutter table, so that won't have any change relative to the defaults I had been assigning, but it would result in real values for all the other items, like source_name (1345_-70), alias, ra, dec, and stellarity.
Good catch!!
There was a problem hiding this comment.
Latest commits have updated this function to get source info from the MSA source table for both "real" (source_id > 0) and virtual (source_id < 0) sources. No more zeroing out source-related info for slits with virtual sources.
|
Latest regtest run started at https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1482 |
|
Latest regtest run results show only expected differences in source-related keyword values for virtual slits, as well as slight differences in the PATHLOSS_PS array values for virtual slits, because they now have source position info. Also just a few unrelated differences due to other recent PR merges. So the whole run looks good. Ready for a final review. |
|
@nden Do you want to have another look at this yet? |
|
I'll review today |
|
Something in the code was reverted and now the names of the virtual slits are the same as on master. On the positive side I see source_ra/dec for the virtual slits populated with the data from the msa file. |
Yes, that's expected. The latest version of the code no longer does any "tweaking" of the source info for virtual slits, but instead just uses all the entries from the source table in the MSA file, so in essence they're treated exactly the same as regular source slits, hence we would not expect any difference in things like source name, alias, etc. compared to the existing code on master. The only special handling that virtual slits get now is to remove the negative sign from the source_id value when the source_id gets used to create the level-3 file names. Everything else, including their negative source_id's, funky source names and aliases, etc. are passed along unchanged from what's in the MSA file. |
Resolves JP-2922
Closes #7287
This PR updates all the source_id handling machinery within associations, calwebb_spec2 steps, and the calwebb_spec3 pipeline to handle the new source_id syntax for level-3 source-based products, which includes an increase in the number of source_id digits from 5 to 9, and implements the use of "b" and "v" source prefixes for NIRSpec MOS data extracted from background and virtual slits, respectively.
Note that the exp_to_source routine has been updated to use "source_name" as the key for sorting MOS datamodels, because it's the only piece of meta data available that properly distinguishes between "source", "background", and "virutal" slits. source_id values can overlap with one another for MOS now (e.g. a "real" source slit can have the same source_id value as a background slit). Other multi-slit modes (NIRSpec FS and NIRCam/NIRISS WFSS) still use source_id for sorting in exp_to_source, because they don't have source_name populated (names are unknown).
Checklist for maintainers
CHANGES.rstwithin the relevant release sectionHow to run regression tests on a PR