Repository navigation
Further performance improvements for earth-centred transformations #11072
Description
#11069 sped up the calculation of GCRS obsgeoloc and obsgeovel from EarthLocation for the GCRS<->(CIRS, TETE) transforms. It may be possible to
- Use the same technique to (slightly?) speed up
erfa_astrom.apco(see Add topocentric CIRS and fix the CIRS to AltAz transformations #10994 (comment)) - Speed up
EarthLocation.get_gcrs(). (see Speed up EarthLocation.get_gcrs #11073) - Be cleverer in the transformations, not using
FunctionTransformWithFiniteDifferencewhen all that is being done is introduce a rotating frame. EDIT: this would work only ifobstimewere the same; can one special-case this? EDIT 2: best to just special-case "interesting" transformations like ICRS->AltAz, as suggested in Implement direct ICRS -> Observed transform #10887.
Activity
[-]Further performance improvements for GCRS[/-][+]Further performance improvements for earth-centred transformations[/+]mhvk commentedon Nov 23, 2020
@mkbrewer, @StuartLittlefair - a question on possible further speed-ups: the reason location->GCRS made such a difference is that in the current framework, each transformation is actually done 3 times, in order to also find any frame-dependent velocity offsets. For ITRS->CIRS/TETE these are actually known, so it was easy, and for CIRS/TETE->GCRS any induced apparent velocity change due to polar motion is tiny (at least for objects actually close to the Earth). Are there other transformations were one similarly can either calculate the velocity or know any change is negligible so one can speed them up? IIRC,
FunctionTransformWithFiniteDifferencewas always meant to be a bit of a placeholder, just to ensure velocities worked. Can we substitute any withFunctionTransform?p.s. Of course, maybe other things are more important, such as actually having BCRS...
StuartLittlefair commentedon Nov 23, 2020
Yes, the short answer is that all the transforms “downstream” of GCRS can be sped up as long as the obstime is the same in the source and destination frames. That’s TETE, TEME, PrecessedGeocentric (which should be MEME for consistency) CIRS and ITRS.
In this special case the transforms are simple matrix rotations - no need for a FunctionTransform at all. It’s why you could simplify the EarthLocation code - the transforms you were writing were all explicitly at the same obstime.
This would require re-jigging the transform machinery so transforms where the obstime changes follow a different path through the transform graph.
Once the obstime changes, then all bets are off, because I can’t see a way to do the transforms without backing out to ICRS. Maybe if we thought long and hard the GCRS-ICRS transform could eventually done without the finite differencing. This would require many of the ERFA routines to be velocity aware I think. A big job.
mhvk commentedon Nov 23, 2020
OK, that makes sense. I think that then ends up with your earlier conclusion, that the easiest progress is to introduce special paths for common transformations like
ICRS->AltAzwhere along the path we can use thatobstimedoes not change.But I can also see expanding (or subclassing)
FunctionTransformWithFiniteDifferenceto allow the function to return a flag of whether the finite differencing is actually necessary.astrojuanlu commentedon Nov 25, 2020
Note to self: an attempt to do that can be found at #10404
astrojuanlu commentedon Nov 25, 2020
@StuartLittlefair do you think it's worth it to add an alias?
mkbrewer commentedon Nov 25, 2020
I'm afraid that I don't understand this. GCRS and BCRS are both aligned with the ICRF, so velocities don't change when transforming between the two. It's only when one does the BCRS->ICRS->BCRS transform at different obstimes that velocities change. For stars and planets, at least, the relevant change is only to the velocity of the observer which is calculated from the sum of the velocity of the Earth and that of observer in the GCRS, both of which are known at the new obstime, so where do finite differences come in here? The only case that I know of where they would is the case of an observer in space where only the observer's position as a function of time is known. Is that what you are referring to? Or am I missing something?
StuartLittlefair commentedon Nov 25, 2020
What I mean here is that the transform between ITRS(t1) and CIRS(t1) is a simple matrix rotation. But the transformation between ITRS(t2) and CIRS(t1) is not just a matrix transform, but a more complex function.
In astropy the transforms are either marked as some sort of matrix transform or affine transform, or a transform that is some sort of black box function. The matrix transforms can apply the same matrix to positions and velocities so are quick, simple and accurate.
The black box functions calculate velocities using a finite difference approach so are slower and less precise. But we do have another type of transform, which is a
FunctionTransformthat knows how to handle velocities - which could treat the velocities the way you suggest.We could move to a system where every frame "downstream" had to go through a GCRS frame at the same obstime via
DynamicMatrixTransforms. The GCRS->ICRS transform could then be aFunctionTransformthat calculated the velocities as you suggest.It would mean removing any transformations from "downstream" frames to frames "upstream" of GCRS.
mkbrewer commentedon Nov 25, 2020
Well, in order to do that transform, one must got back to the ICRS, undoing aberration and gravitational light deflection using the original obstime, then go forward again looking up the velocity of the Earth and the observer in the GCRS in order to apply aberration and gravitational light deflection at the new obstime. So, I still don't see any application of finite differencing there.
StuartLittlefair commentedon Nov 25, 2020
FiniteDifferencing is used because we do not (yet) have a function to transform the velocities of a coordinate in such a transform.
So to calculate the coordinate (with velocities) in CIRS(t1), we calculate CIRS(t1 + dt/2) and CIRS(t1 - dt/2) and use the difference in positions to calculate the velocity. This is what is meant by the finite differencing.
mkbrewer commentedon Nov 25, 2020
Oh, I see. So you want the relative velocity of the target wrt to the observer then, even though this is not used in any of these transforms. In the case of planets, this could be obtained from the ephemeris in addition to the target position. In the case of stars, it's a bit more complicated as you have both a constant radial velocity and some lateral velocity implied by proper motion.
mhvk commentedon Nov 25, 2020
Indeed, we need the apparent velocity induced by the frame transform.
@StuartLittlefair - from your description, it does sound like it would be helpful if
FunctionWithFiniteDifferenceor a subclass would allow the transformation function to flag that it had already calculated the induced velocity (see #11072 (comment)) - for quite a few transformations,obstimenot changing would seem to be the most common case.StuartLittlefair commentedon Nov 25, 2020
If it’s possible to calculate the induced velocity another way why use FunctionTransformWithFiniteDifference at all?
Why not write a FunctionTransform that returns the correct velocity.
mkbrewer commentedon Nov 25, 2020
Well, in the case of stars, this seems simple enough. If you have a distance, radial velocity and proper motion in right ascension and declination, you can calculate the lateral velocities from these without having to resort to finite differences. Then you can transform this velocity into the CIRS using the BPN.
mhvk commentedon Nov 25, 2020
@StuartLittlefair - The choice would only be for cases where whether it is easy or not depends on whether
obstimein the two frames is the same, i.e., it would be a way to leave the decision on how to calculate it to the function (rather than to the transformgraph).StuartLittlefair commentedon Nov 25, 2020
I see what you mean @mhvk. For example,
itrs_to_CIRScould special case the same obstime case, calculate the velocities and indicate toFunctionTransformWithFiniteDifferencethat it should act differently and skip the finite difference bit.I think both approaches have their merits.
Reorganising the graph and making GCRS-ICRS a
FunctionTransformseems tidier and would give good precision even if the obstime was changing. But it’s a big change and we’d have to be careful the velocity calculation was done correctly.If we took your approach we need a way to “talk” to the
FunctionTransformWithFiniteDifference. Only the “from” coordinate and the function itself are available so perhaps we would set a private attribute on the function when it is first called? This feels a bit hacky, so perhaps there’s a better way.mhvk commentedon Nov 26, 2020
I definitely like doing the correct calculation all the time better, though obviously one needs to be careful.
For
FunctionTransformWithFiniteDifference, I was thinking the other way around: the actual function called can return something extra if it is able to calculate the velocities (e.g. a tuple of position and velocity), and then the FTWFD class can recognize that. But perhaps a subclass explicitly written for flexibility is better - so that, e.g., one can just pass on the velocities as well.mkbrewer commentedon Nov 26, 2020
While the coordinate transforms don't currently deal with the target's velocity, they could be upgraded to do so. For instance, the ICRS->GCRS transform could deal with velocity in the same way that it does for position. The
ErfaAstrommethodapcs()returns the observer's velocity wrt the SSB inastrom['v'](as doesapco()). This can be subtracted from the ICRS target velocity in the same way asastrom['eb']is subtracted from the position to form the GCRS velocity of the target.mkbrewer commentedon Nov 28, 2020
This is a bit off topic, but with this talk about velocities I got to thinking about how I handle these in my tracking programs for spectroscopy. This led me to your
SpectralCoordclass. There I see examples of detected line frequencies being converted into radial velocities, but I don't see the opposite. Say I want to observe the 1-0 transition of 12CO in the Orion molecular cloud given its velocity of ~9 km/s wrt the LSR. Is there a way using theSpectralCoordclass to obtain the frequency on the sky at that velocity in the rest frame of the observer so that I can set my local oscillator frequency such that 9 km/s sits midband in my spectrometer?mkbrewer commentedon Nov 28, 2020
@mhvk @StuartLittlefair Please see my comment in PR #11073 (comment) regarding improved accuracy (on my side this time). I posted there as it seemed most appropriate, but I do realize that people don't often look at merged PRs.
mhvk commentedon Dec 15, 2020
@mkbrewer - belatedly, not super familiar myself with the new
SpectralCoordclass capabilities, so let me ping @astrofrog about your #11072 (comment)My guess is that it is possible - if so, it sounds like it would be good to have an actual example in the docs!