Open jhod0 opened 3 months ago
Thanks @kbwestfall , I added a short description under "Instrument-Specific Updates" in that file.
Should this change also be applied to the GMOS-North implementation? Right now this is only on GMOS-South. I have no GMOS-North data to test this on, but I would hope the mask design stuff is identical between the two instruments.
Nevermind - the update is in the parent class of both GMOS-South and GMOS-North, so it is already applied to both.
Hi @jhod0 . Thanks! This looks good to me. Two requests:
- Can you add a note on this in the
doc/releases/1.16.1dev.rst
doc?- We should check with @debora-pe about the offset you see in the predictions vs. the measurements. I expect this should be okay because the code should determine this offset. I.e., the exact prediction doesn't really matter, as long as the code is able to correctly match the slit mask design to the slits detected in the image.
Hi @jhod0 Thank you for doing this. @kbwestfall is right in that the offset is not a problem as long as the cross correlation between measured and predicted is correct. We should still test these changes with other Gemini-N/S datasets. We have several in our Dev Suite. If that is not possible for you @jhod0, we can do it (I may be able to run the test sometime next week).
Great! I certainly can test on other GMOS data sets, but it will be on the back burner for me and I might not get to it for a few weeks.
I run the tests. All good (the few failures are not related)
Test Summary
--------------------------------------------------------
--- PYTEST PYPEIT UNIT TESTS FAILED 3 failed, 253 passed, 66 warnings in 339.50s (0:05:39) ---
--- PYTEST UNIT TESTS FAILED 2 failed, 146 passed, 166 warnings in 1625.44s (0:27:05) ---
--- PYTEST VET TESTS PASSED 61 passed, 97 warnings in 5141.07s (1:25:41) ---
--- PYPEIT DEVELOPMENT SUITE PASSED 239/239 TESTS ---
Testing Started at 2024-07-18T15:58:02.563299
Testing Completed at 2024-07-19T04:58:47.612689
Total Time: 13:00:45.049390
Unfortunately, we have only one dataset in the Dev Suite that uses the mask definition information and it's GMOS-S with non-tilted slits. So, I could not check much, but the results for the non-tilted slits data did not change, so I guess it's good thing :) So, I think this can be merged.
Currently the GMOS mask design info in PypeIt does not allow for tilted slits, this PR tries to implement that. @SunilSimha had given me some quick instructions on what to change for this.
I believe this is now working for GMOS-South with data of mine taken in January with the newly-installed detectors. I have managed to do PypeIt reductions with these changes which appear, to me, to match up slits correctly with their mask design info.
One small "issue" is that the predicted and empirically measured slit edges seem to be systematically off by ~10pix. Is this a negligible offset, or something that actually needs to be corrected? In any case, it appears to work the same for tilted and non-tilted slits.
The following is from a slit mask where the slits centered at ~533 and ~652 are tilted. Here are the measures slit edges for one mask, loaded in from a
Slits....tgz
calibration file:And here are the predicted slit edges from
get_maskdef_slitedges()
, which I printed out from planting apdb.set_trace()
and during a regular PypeIt reduction.