# astrometry has a bug in retrieving TAN_PIXELS and it exposes the need for a better API

XMLWordPrintable

## Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Team:

## Description

meas_astrom astrometry.py has two errors in how it retrieves the TAN_PIXELS coordinate system:

 pixelsToTanPixels = exposureInfo.getDetector().getTransformMap().get(TAN_PIXELS) if pixelsToTanPixels:  ...

The first error is using a CameraSysPrefix instead of a CameraSys to specify the transform; the second error is in assuming get will return None if the system is not present. Unfortunately the correct code is ugly and suggests the need for a better API:

 detector = exposureInfo.getDetector() tanPixCamSys = detector.getCameraSys(TAN_PIXELS) if tanPixCamSys in detector.getTransformMap():  pixelsToTanPixels = detector.getTransformMap().get(tanPixCamSys)  ...

TransformMap can only handle CameraSys and fixing that would be hard, so I propose instead to add two overloaded methods to Detector, both of which can accept a CameraSys or a CameraSysPrefix:

• getTransform(sys)
• hasTransform(sys)

The resulting code will be:

 detector = exposureInfo.getDetector() if detector.hasTransform(TAN_PIXELS):  pixelsToTanPixels = detector.getTransform(TAN_PIXELS)  ...

## Activity

Hide
Russell Owen added a comment -

The work is on tickets/DM-2471 of afw and meas_astrom

The afw changes consist of adding two member functions to Detector, both of which can take a CameraSys or a CameraSysPrefix:

• hasTransform
• getTransform
I expanded tests/testDetector.py to test these new member functions.

The meas_astrom changes consist of rewriting the code that creates the DistortedTanWcs if the necessary information is available. Unfortunately that is not yet tested (and was not before). I propose making such a test a new ticket in order to move this along (meanwhile Dominique can test it directly with his CFHT data), but I can postpone if need be.

Show
Russell Owen added a comment - The work is on tickets/ DM-2471 of afw and meas_astrom The afw changes consist of adding two member functions to Detector, both of which can take a CameraSys or a CameraSysPrefix: hasTransform getTransform I expanded tests/testDetector.py to test these new member functions. The meas_astrom changes consist of rewriting the code that creates the DistortedTanWcs if the necessary information is available. Unfortunately that is not yet tested (and was not before). I propose making such a test a new ticket in order to move this along (meanwhile Dominique can test it directly with his CFHT data), but I can postpone if need be.
Hide
Russell Owen added a comment - - edited

Simon: please try again. I accidentally branched afw from another branch instead of master. I have corrected that mistake.

I also added a new function getDistortedWcs to afw.image.utils and modified AstrometryTask's run method to call that. This simplifies the code and makes it more robust, since the new function is reusable and tested.

Finally I reduced the default SIP order of the TAN-SIP fitter from 5 to 3, to improve robustness. The fitter can easily go unstable at higher orders.

Show
Russell Owen added a comment - - edited Simon: please try again. I accidentally branched afw from another branch instead of master. I have corrected that mistake. I also added a new function getDistortedWcs to afw.image.utils and modified AstrometryTask's run method to call that. This simplifies the code and makes it more robust, since the new function is reusable and tested. Finally I reduced the default SIP order of the TAN-SIP fitter from 5 to 3, to improve robustness. The fitter can easily go unstable at higher orders.
Hide
Kian-Tat Lim added a comment -

Looking at the old branch, the only comment I have is that you should use "Does ... exist" rather than "Does ... exists".

Show
Kian-Tat Lim added a comment - Looking at the old branch, the only comment I have is that you should use "Does ... exist" rather than "Does ... exists".
Hide
Simon Krughoff added a comment -

As long as this passes a buildbot build, it looks good to me.

Show
Simon Krughoff added a comment - As long as this passes a buildbot build, it looks good to me.

## People

• Assignee:
Russell Owen
Reporter:
Russell Owen
Reviewers:
Simon Krughoff
Watchers:
Dominique Boutigny, Kian-Tat Lim, Russell Owen, Simon Krughoff