Skip to content

Update precision test of AltAz computation. - #11103

Merged
StuartLittlefair merged 1 commit into
astropy:masterfrom
mhvk:coord-alt-az-test-update-reference-position
Jan 4, 2021
Merged

StuartLittlefair merged 1 commit into
astropy:masterfrom
mhvk:coord-alt-az-test-update-reference-position

Conversation

@mhvk

@mhvk mhvk commented Nov 30, 2020

Copy link
Copy Markdown
Contributor

Follow-up of #11073 (comment), where @mkbrewer found that astropy and his code are now consistent down to well below 100 nas. Updated the test accordingly, fixing a minor precision-reducing bug in the process.

@mhvk mhvk added testing coordinates Affects-dev PRs and issues that do not impact an existing Astropy release precision labels Nov 30, 2020
@mhvk mhvk added this to the v4.3 milestone Nov 30, 2020
@github-actions github-actions Bot removed the testing label Nov 30, 2020
@StuartLittlefair

Copy link
Copy Markdown
Contributor

Hi @mhvk

Thanks for this update. I'm wondering if we are doing the right thing by tightening the precision so much. The two codes are both implementations of SOFA algorithms that include various treatments of precession etc. They agree very precisely which is wonderful, but should these tests tie us to these algorithmic choices?

Perhaps I haven't explained myself very well?

Comment thread astropy/coordinates/tests/test_intermediate_transformations.py Outdated
@mkbrewer

Copy link
Copy Markdown
Contributor

Well, my objective here has been to get Astropy up to the full accuracy of the bare SOFA model, which we have now accomplished. There are several improvements that you could make to this model that would invalidate this test, the first of which would be to include the effects of free core nutation provided by the dX and dY values given in the IERS tables. For a good introduction to these effects see Chapter 5 here:

https://www.iers.org/IERS/EN/Publications/TechnicalNotes/tn36.html

@mhvk

mhvk commented Nov 30, 2020

Copy link
Copy Markdown
Contributor Author

@mkbrewer - yes, good point. I started a new issue about possible further improvements: #11110.

@StuartLittlefair - I pushed the PR with the revised note.

@mhvk
mhvk force-pushed the coord-alt-az-test-update-reference-position branch from 5ad4710 to 64d941c Compare November 30, 2020 18:05
@mhvk

mhvk commented Nov 30, 2020

Copy link
Copy Markdown
Contributor Author

Annoying that the remote-data run fails because of a numpy-dev problem...

@mhvk

mhvk commented Nov 30, 2020

Copy link
Copy Markdown
Contributor Author

But to be clear, the test passed locally and (I think...) also passed before here.

@StuartLittlefair

Copy link
Copy Markdown
Contributor

Looks like a problem reading the IERS table?

@mhvk

mhvk commented Nov 30, 2020

Copy link
Copy Markdown
Contributor Author

The test failed very quickly, with

E   RuntimeWarning: numpy.ndarray size changed, may indicate binary incompatibility. Expected 80 from C header, got 88 from PyObject

@mkbrewer

mkbrewer commented Dec 5, 2020

Copy link
Copy Markdown
Contributor

Should these tests be rerun now that #11116 has been merged?

@mhvk
mhvk force-pushed the coord-alt-az-test-update-reference-position branch from 64d941c to 6ba4bec Compare December 29, 2020 00:16
@mhvk

mhvk commented Dec 29, 2020

Copy link
Copy Markdown
Contributor Author

Rebased to see if tests now pass. @StuartLittlefair - let me know if you have any remaining comments.

@StuartLittlefair
StuartLittlefair merged commit 3c7d547 into astropy:master Jan 4, 2021
@StuartLittlefair

Copy link
Copy Markdown
Contributor

Thanks @mhvk!

@mhvk
mhvk deleted the coord-alt-az-test-update-reference-position branch January 4, 2021 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Affects-dev PRs and issues that do not impact an existing Astropy release coordinates precision Ready-for-final-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants