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

Simplify rotSkyPos calculation #469

Merged
merged 4 commits into from
May 14, 2024
Merged

Simplify rotSkyPos calculation #469

merged 4 commits into from
May 14, 2024

Conversation

jmeyers314
Copy link
Member

Updates readout.py:compute_rotSkyPos.

This routine had previously instantiated a complete WCS, marched separate test points a little north, and a little towards the CCD +Y to determine RotSkyPos empirically for insertion into the image header. The new routine follows notes I made at https://smtn-019.lsst.io/v/DM-44258/index.html, developing something closer to an analytic formula for determining RotSkyPos. The key insight is that RotSkyPos is intended to relate the camera orientation to ICRF north and not true north. Previous attempts I've seen at analytic relations use the telescope rotator angle (rotTelPos) and the parallactic angle, the latter being the position angle of zenith measured from true North (not ICRF North). This is the wrong north! So I invented the pseudo-parallactic angle which just swaps true north -> ICRF north. With that, plus acknowledging a few realities about how telescopes flip images around and other coordinate conventions, the analytic formula for Rubin becomes:

RotSkyPos = 270 - RotTelPos + pseudo-parallactic-angle.

I also clarified the documentation a bit, and fixed one line where the MJD was being interpreted in the (default) UTC time scale instead of the TAI scale.

Note that in the end, I've concluded we didn't make a mistake in simulating OR3. We were correct to use the regular parallactic angle inside batoid_wcs since the place where we use it relates true-of-date "observed" ra/dec coordinates with altitude and azimuth. A separate transformation handles true north <-> ICRF north, so we didn't need the pseudo parallactic angle for creating the WCS.

@jmeyers314 jmeyers314 requested a review from jchiang87 May 11, 2024 00:52
Copy link
Collaborator

@jchiang87 jchiang87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, though I'm wondering how we should test this. For the older implementation, I simulated a handful of CCDs around the periphery of the focal plane for a couple visits, and ran single frame processing through calibrate to check the astrometry. It would be good to add some tests within the scope of this package.

@jmeyers314
Copy link
Member Author

New code is still subject to the old test: https://github.com/LSSTDESC/imSim/blob/main/tests/test_readout.py#L198

If you think we need more than this though, I can probably figure out how to do a one-off calibrate.

@jchiang87
Copy link
Collaborator

Ah, I forgot about that test (even though I wrote it). I guess it's fine, for now. It would be good to get rid of LsstCamImSim entirely, but that doesn't have to happen here.

@jmeyers314 jmeyers314 merged commit 3311004 into main May 14, 2024
3 checks passed
@jmeyers314 jmeyers314 deleted the simplify_rotskypos branch May 14, 2024 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants