Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-20133

VisitInfo uses incorrect calculation of hour angle

    XMLWordPrintable

    Details

    • Story Points:
      4
    • Team:
      Architecture

      Description

      The calculations of hour angle and parallactic angle in VisitInfo.cc use getBoresightRaDec(), which returns Ra and Dec in the  International Celestial Reference System (ICRS). However, the correct frame for those calculations is the Celestial Intermediate Origin (CIO, or cirs in Astropy). Using the wrong coordinate frame ignores precession and nutation. It is probably correct to store the boresight RA and Dec coordinates in ICRS, but they need to be transformed to CIO for the hour angle and parallactic angle calculations.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            I think it's also possible that we need to apply an additional correction because we are using earth rotation angle and not local sidereal time.

            Show
            tjenness Tim Jenness added a comment - I think it's also possible that we need to apply an additional correction because we are using earth rotation angle and not local sidereal time.
            Hide
            tjenness Tim Jenness added a comment -

            There is other code that gets this wrong in the stack. For example https://github.com/lsst/obs_subaru/blob/master/python/lsst/obs/hsc/hscPupil.py#L70 tries to calculate hour angle from a VisitInfo and gets it wrong and this code should be fixed to call the VisitInfo method directly (and it should not be calculating the parallactic angle itself either).

            I think that's the only other place that seems to be using getEra outside of test code.

            Show
            tjenness Tim Jenness added a comment - There is other code that gets this wrong in the stack. For example https://github.com/lsst/obs_subaru/blob/master/python/lsst/obs/hsc/hscPupil.py#L70 tries to calculate hour angle from a VisitInfo and gets it wrong and this code should be fixed to call the VisitInfo method directly (and it should not be calculating the parallactic angle itself either). I think that's the only other place that seems to be using getEra outside of test code.
            Hide
            sullivan Ian Sullivan added a comment -

            Some notes from a brief conversation with John Parejko: The ticket that introduced the incorrect calculation was DM-8052. The unit test set up in VisitInfoTestCase (in afw/tests/test_visitInfo.py) should be modified to be self-consistent, and the test function computeLstHA should be written using astropy coordinate transformations, which should break the hour angle unit test. In our brief discussion we didn't see an obvious way to fix the calculation of hour angle in VisitInfo.cc, though.

            Show
            sullivan Ian Sullivan added a comment - Some notes from a brief conversation with John Parejko : The ticket that introduced the incorrect calculation was DM-8052 . The unit test set up in VisitInfoTestCase  (in  afw/tests/test_visitInfo.py ) should be modified to be self-consistent, and the test function computeLstHA  should be written using astropy coordinate transformations, which should break the hour angle unit test. In our brief discussion we didn't see an obvious way to fix the calculation of hour angle in VisitInfo.cc , though.
            Hide
            tjenness Tim Jenness added a comment -

            Fixing the C++ is hard without pulling in our own copy of ERFA and doing it the hard way. We could add the methods from python via visitInfoContinued.py and use astropy in there. Having people work it out themselves has been demonstrated to be a bad idea and we have at least one package that is trying to do this outside of tests.

            Show
            tjenness Tim Jenness added a comment - Fixing the C++ is hard without pulling in our own copy of ERFA and doing it the hard way. We could add the methods from python via visitInfoContinued.py and use astropy in there. Having people work it out themselves has been demonstrated to be a bad idea and we have at least one package that is trying to do this outside of tests.
            Hide
            Parejkoj John Parejko added a comment -

            Jointcal calls getBoresightHourAngle and getEra in C++ for each SourceCatalog (as part of a refraction angel calculation), but I could lift that up into python if the method is lifted out of C++ in order to make it more correct.

            Show
            Parejkoj John Parejko added a comment - Jointcal calls getBoresightHourAngle and getEra in C++ for each SourceCatalog (as part of a refraction angel calculation), but I could lift that up into python if the method is lifted out of C++ in order to make it more correct.

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              sullivan Ian Sullivan
              Watchers:
              Ian Sullivan, John Parejko, John Swinbank, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated: