Add nut06a computation to ErfaAstromInterpolator by maxnoe · Pull Request #20416 · astropy/astropy

maxnoe · 2026-09-16T13:59:32Z

This may be used to speedup transformations involving the GeocentricTrueEcliptic frame for arrays of equinoxes.

Description

This pull request is to address #20415

Fixes #20415

AI Disclosure

If AI tools were used to develop this pull request, describe the tools including specific model and version, how they were used, and what content is AI generated. Otherwise enter "N/A".

OpenAI Codex using gpt-6-astra was used for identifying the particular functions needed to address the performance issue in the example of #20415 and then make the addition of nut06 to the erfa_astrom machinery.

I refactored the model output to make it more explicit and direct.

The tests were also written by the model.

I certify that I am human and that I take full responsibility for this pull request including all interactions with reviewers.

Merge method

By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

github-actions · 2026-09-16T13:59:45Z

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

Do the proposed changes actually accomplish desired goals?
Do the proposed changes follow the Astropy coding guidelines?
Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

maxnoe · 2026-09-16T14:06:16Z

Example:

import numpy as np
from astropy import units as u
from astropy.coordinates import GeocentricTrueEcliptic, SkyCoord
from astropy.time import Time
from astropy.coordinates.erfa_astrom import erfa_astrom, ErfaAstromInterpolator

erfa_astrom.set(ErfaAstromInterpolator(5 * u.min))

n = 1_000_000
obstime = Time("2025-01-01") + np.linspace(0, 2, n) * u.hour
target_coord = SkyCoord(
    np.linspace(0, 360, n, endpoint=False) * u.deg,
    0 * u.deg,
    obstime=obstime,
)
target_coord.transform_to(GeocentricTrueEcliptic(equinox=obstime))

Here:

❯ time python trafo.py
python trafo.py  2.43s user 0.57s system 157% cpu 1.908 total

on main:

❯ time python trafo.py
python trafo.py  40.21s user 0.30s system 100% cpu 40.243 total

pllim · 2026-09-16T15:16:39Z

You can ignore the 4 failures in devdeps jobs about np.minmax. FYI.

maxnoe · 2026-09-16T15:34:36Z

I found a couple of other trafos also using nut06a, would you prefer a separate PR for those or should I add the changes here?

pllim · 2026-09-16T15:43:23Z

would you prefer a separate PR for those or should I add the changes here?

I have no preference as I am not a coordinates maintainer. I trust your judgement.

maxnoe · 2026-09-16T16:04:25Z

@mhvk maybe wants to have a look?

maxnoe · 2026-09-16T16:23:33Z

Added the rest of the transforms, little benchmark:

import numpy as np
from astropy import units as u
from astropy.coordinates import GeocentricTrueEcliptic, SkyCoord, ICRS, TETE, CIRS, EarthLocation, ITRS, GCRS
from astropy.time import Time
from astropy.coordinates.erfa_astrom import erfa_astrom, ErfaAstromInterpolator
from time import perf_counter

erfa_astrom.set(ErfaAstromInterpolator(5 * u.min))

n = 100_000
t0 = Time("2025-01-01")
obstime = t0 + np.linspace(0, 2, n) * u.hour

combinations = [
    (ICRS(), GeocentricTrueEcliptic(equinox=obstime)),
    (GCRS(), CIRS()),
    (GCRS(), TETE()),
    (TETE(), ITRS()),
]


coord = SkyCoord(
    np.linspace(0, 360, n, endpoint=False) * u.deg,
    0 * u.deg,
    obstime=obstime,
)

for from_frame, to_frame in combinations:
    from_coord = coord.transform_to(from_frame)

    t0 = perf_counter()
    to_coord = from_coord.transform_to(to_frame)
    t = perf_counter() - t0

    print(f"{from_frame.__class__.__name__} -> {to_frame.__class__.__name__}: {t:5.2} s")

❯ python trafo.py
ICRS -> GeocentricTrueEcliptic: 0.066 s
GCRS -> CIRS:   3.2 s
GCRS -> TETE:   3.0 s
TETE -> ITRS:  0.16 s
❯ git switch main
Switched to branch 'main'
Your branch is up to date with 'upstream/main'.
❯ python trafo.py
ICRS -> GeocentricTrueEcliptic:   3.4 s
GCRS -> CIRS:   7.1 s
GCRS -> TETE:   6.5 s
TETE -> ITRS:   3.7 s

mhvk

@maxnoe - this looks good. Mostly very minor comments, except perhaps to move _precession_nutation_matrix to utils.py so it can be used in all places (at least, if I understood correctly).

Also, a question just to be sure: might it make sense to have _precession_nutation_matrix be on ErfaAstrom? It would break the idea that it interpolates specific erfa routines, though, so I think what you have is better, but I thought I would bring it up anyway...

mhvk · 2026-09-16T16:26:17Z

    jd1, jd2 = get_jd12(time, "tt")
    if rbpn is None:
-        # erfa.gst06a calls pnm06a to calculate rbpn and then gst06. Use it in
-        # favour of getting rbpn with erfa.pnm06a to avoid a possibly large array.


So, I guess we're giving up the "possibly large array" - I think that's OK, just to confirm.

I think we should maybe also just wrap gst06 directly? That would avoid that

maxnoe · 2026-09-16T17:59:24Z

It would break the idea that it interpolates specific erfa routines, though, so I think what you have is better, but I thought I would bring it up anyway...

I thought about something similar, basically wrapping all commonly used erfa methods and letting ErfaAstromInterpolator decide which are worth interpolating and which are not.

This may be used to speedup transformations involving the ``GeocentricTrueEcliptic`` frame for arrays of equinoxes.

maxnoe · 2026-09-17T06:39:59Z

+    rbpn = erfa_astrom.get().pnm06a(time)
+    x, y = erfa.bpn2xy(rbpn)
+    s = erfa.s06(*get_jd12(time, "tt"), x, y)
+    return erfa.c2ixys(x, y, s)


@mhvk Isn't this also just the equivalent of get_cip? I.e. get x, y, s and then transform to the matrix?

I think I will add get_cip then to erfa_astrom directly

mhvk

@maxnoe - I think this looks good. For this PR, I suggest we try to keep it simple, and just speed up those true ecliptic transformations. Some other comments in-line.

mhvk · 2026-09-20T23:39:40Z


        return astrom

+    def pnm06a(self, time, return_obl=False):


I'm still wavering a bit about whether it makes sense to supply also functions that are not overridden. On the other hand, this one joins the others in that the input is an astropy Time rather than a TT jd1, jd2 pair, and of course in returning the obliquity.

So, overall this seems good.

However, can you just return the obliquity unconditionally? I've slowly been convinced that having the number of outputs depend on inputs is bad form (array API specifically avoids it), and there is no extra calculation involved, so one can just do rnpb, _ = erfa_astrom.get().pnm06a(...). Possibly this means one should give it a different name, though.

mhvk · 2026-09-20T23:46:29Z

@@ -0,0 +1,11 @@
+Added ``nut06a`` to ``ErfaAstrom`` and ``ErfaAstromInterpolator`` and


We need to mention pnm06a (and perhaps remove nut06a, see earlier comment).

More generally I wonder if this is not too much implementation detail. One could also note that when using the interpolator, the true-ecliptic transformations can now be sped up considerably.

github-actions Bot added the coordinates label Sep 16, 2026

maxnoe force-pushed the nut-erfa-astrom-interpolator branch 2 times, most recently from a692354 to 6e2253d Compare September 16, 2026 14:04

maxnoe force-pushed the nut-erfa-astrom-interpolator branch from 6e2253d to 349e7ca Compare September 16, 2026 14:10

pllim added this to the v8.1.0 milestone Sep 16, 2026

pllim added Performance benchmark Run benchmarks for a PR Extra CI Run cron CI as part of PR labels Sep 16, 2026

pllim requested a review from lpsinger September 16, 2026 14:32

maxnoe force-pushed the nut-erfa-astrom-interpolator branch from 1ff8ef2 to b95633a Compare September 16, 2026 14:43

lpsinger approved these changes Sep 16, 2026

View reviewed changes

pllim requested a review from mhvk September 16, 2026 16:18

mhvk reviewed Sep 16, 2026

View reviewed changes

maxnoe added 4 commits September 16, 2026 20:25

Add nut06a computation to ErfaAstromInterpolator

7846fc0

This may be used to speedup transformations involving the ``GeocentricTrueEcliptic`` frame for arrays of equinoxes.

Avoid duplicated computation of obl06

0b6eedf

Add erfa_astrom nut06 support to more transformations

40cbbc1

Address review comments for test utils

6debeb7

maxnoe force-pushed the nut-erfa-astrom-interpolator branch from c5ad09b to 6debeb7 Compare September 16, 2026 18:43

Move _precession_nutation_matrix to erfa_astrom

97665f6

maxnoe commented Sep 17, 2026

View reviewed changes

maxnoe mentioned this pull request Sep 17, 2026

Use broadcast_shapes instead of broadcast in GCRS equality checks #20420

Merged

2 tasks

mhvk reviewed Sep 20, 2026

View reviewed changes

		@@ -0,0 +1,11 @@
		Added ``nut06a`` to ``ErfaAstrom`` and ``ErfaAstromInterpolator`` and

Uh oh!

Conversation

maxnoe commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

AI Disclosure

Merge method

Uh oh!

github-actions Bot commented Sep 16, 2026

Uh oh!

maxnoe commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pllim commented Sep 16, 2026

Uh oh!

maxnoe commented Sep 16, 2026

Uh oh!

pllim commented Sep 16, 2026

Uh oh!

maxnoe commented Sep 16, 2026

Uh oh!

maxnoe commented Sep 16, 2026

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mhvk Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

maxnoe Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maxnoe commented Sep 16, 2026

Uh oh!

maxnoe Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

mhvk Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

mhvk Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maxnoe commented Sep 16, 2026 •

edited

Loading

maxnoe commented Sep 16, 2026 •

edited

Loading