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

Kl/update lss traces #168

Merged
merged 6 commits into from
Apr 9, 2024
Merged

Kl/update lss traces #168

merged 6 commits into from
Apr 9, 2024

Conversation

astronomyk
Copy link
Collaborator

As part of to #167, here are the new FITS files for the LSS traces based on the optical model from 2023-06-20 that was used for manufacturing the LSS grisms.

@astronomyk astronomyk self-assigned this Apr 8, 2024
@astronomyk astronomyk added the data update New version of or change to a data file label Apr 8, 2024
@astronomyk astronomyk linked an issue Apr 8, 2024 that may be closed by this pull request
2 tasks
@teutoburg
Copy link
Contributor

I'm not sure I understand the commit history here. Why does this go back 2 years? I checked out a local version of this branch and it let me rebase onto dev_master without any conflicts. Is there a specific reason why the commit history is what it is here? If not, I can push the rebased version to this PR if you want.

@teutoburg
Copy link
Contributor

Also now merging this will close #167, but this PR is only one of two parts there. So I suggest waiting with this merge until the other task is finished, which should be today anyway...

MICADO/test_micado/test_micado_spec.py Outdated Show resolved Hide resolved
WFC3/test_wfc3/test_full_package_wfc3_ir.py Outdated Show resolved Hide resolved
@teutoburg teutoburg merged commit 7039017 into dev_master Apr 9, 2024
8 checks passed
@teutoburg teutoburg deleted the kl/update_lss_traces branch April 9, 2024 11:41
@hugobuddel
Copy link
Collaborator

This broke the METIS notebooks:

Please run the notebooks before you merge something big like this @astronomyk

@hugobuddel
Copy link
Collaborator

The notebooks also run when you start the CI on github manually, so there is no reason to not run them.

@hugobuddel
Copy link
Collaborator

Also note that while TRACE_LSS_L.fits is replaced, with the original being renamed to TRACE_LSS_L_old.fits, it was TRACE_LSS_L_sci.fits that was used as default trace file. Now the default is the new TRACE_LSS_L.fits. I think this is confusing. Is the TRACE_LSS_L_sci.fits also old? I think old files should be removed; they can be found in the history if necessary.

Comparing TRACE_LSS_L_sci.fits with the new TRACE_LSS_L.fits, it can be seen that the main difference is that these header keywords only appear in TRACE_LSS_L_sci.fits: {'CTYPE1', 'CRVAL1', 'CRPIX2', 'MJDREF', 'CRPIX1', 'CDELT2', 'WCSAXES', 'CRVAL2', 'LATPOLE', 'CUNIT2', 'CTYPE2', 'CDELT1', 'CUNIT1'}. The other differences in the headers is expected, like dates and such.

TRACE_LSS_L_sci.fits is still in the repository, so we could 'fix' the notebook by just explicitly referencing that in !OBS.trace_file, but that seems kinda wrong, since I expect that file to be outdated too.

It is also concerning that the TRACE_LSS_L_sci.fits a the old TRACE_LSS_L.fits could not be used interchangeably.

I don't know where these headers came from. I suppose the best solution would be to add similar headers to the new trace maps. Maybe we can just copy them over? I'm assuming here that the overal structure of the FITS file has not changed.

@hugobuddel
Copy link
Collaborator

The notebook says

Before showing the background subtracted spectra, we convert the pixel numbers to wavelength and spatial position along the slit. This is possible because the current version of Scopesim/METIS by default uses perfectly linear mapping of the spectra onto the detector. The WCS keywords to use are in metis['spectral_traces'].meta.

I have not inspected the files, so I don't know whether the new files also use a "perfectly linear mapping". It seems so, but maybe the numbers are not the same anymore. The traces seem more compact.

@teutoburg
Copy link
Contributor

I think old files should be removed; they can be found in the history if necessary.

agree 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data update New version of or change to a data file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

METIS LSS scripts for Wolfgang
3 participants