astropy / astropy/astropy

Premature broadcasting of coordinates in transforms creates slowdowns

Open
#10,997 11 comments 0 reactions 0 assignees View on GitHub
coordinates Package-expert Performance
Dominant language
Python
Stars
5.3k
Forks
2.2k
Avg merge
1d 18h
Merged PRs (30d)
74

Description

### Description

In summary, when transforming coordinate frames we broadcast before transforming, which ought not to be necessary.

In my PR to add a topocentric frame I noticed some odd behaviour. Suppose obstime is a (N, 1) array of times and we have M targets. Below I create an alt-az frame intended to broadcast against each other:

```python
import numpy as np
from astropy.time import Time
import astropy.units as u
from astropy.coordinates import EarthLocation, CIRS, AltAz, SkyCoord
from astropy.coordinates.tests.utils import randomly_sample_sphere
from astropy.coordinates.erfa_astrom import ErfaAstromInterpolator, erfa_astrom

times = Time('2020-08-21T00:00') + np.linspace(-3*u.hour, 3*u.hour, 1000)
location = EarthLocation.of_site('lapalma')
aa_frame = AltAz(obstime=times[:, np.newaxis], location=location)

# 1000 random locations on the sky
ra, dec, _ = randomly_sample_sphere(200)
coos = SkyCoord(ra, dec)
```

Now, if I convert this to AltAz it is extremely slow. But it's much faster if I add CIRS as an intermediate step explicitly, even though CIRS is the intermediate frame between ICRS and AltAz...

```python
coos.transform_to(aa_frame) # 18 seconds
coos.transform_to(CIRS()).transform_to(aa_frame) # 1 second
```

Note this isn't like this in the current master, but it illustrates the issue, which I believe is this. If you run ```coos.transform_to(aa_frame)``` the first step is to make a CIRS frame. The attributes of this frame are collected from the players in the transform, so it picks up the obstime from the AltAz frame and the positions from the ICRS frame, so the resulting CIRS frame is shape (N, M). This large frame is pushed through a CIRS->CIRS' which causes the slowdown.

In the second instance the CIRS->CIRS' transform is only for a coordinate of shape (M,) which is much faster.

Although this specific example doesn't affect astropy master, it will if my PR is merged, and @mhvk points out it probably also slows down a single ICRS position -> AltAz at many times transform.

### Additional context

Contributor guide

Open the contributing guide

Research direction

Start by running the issue's Python reproduction with SkyCoord.transform_to, CIRS, and AltAz to compare the two timings. Trace how the intermediate CIRS frame is assembled and shaped during the direct transform; done means avoiding the unnecessary larger broadcast while preserving the expected coordinate transformation and improving the reported slowdown.

Written by the indexing model from the issue text.

Assessment

Tech stack
numpy, python
Domain
performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.