Repository navigation
Cannot directly transform output from GCRS to AltAz / need ITRS location? #10996
Description
EDIT: put a milestone of 4.3 for the feature request part of this issue, which is to normalize they arguments used to indicate an observer location.
EDIT 2: update to include more recent behaviour (#11069)
Following discussion in #10994, I thought the example below should work, where I use get_body() to get a SkyCoord with a GCRS position for Mars at a given time and location, and try to transform it to AltAz just using body.altaz.
However, it fails, because the temporary AltAz frame that was created does not have a .location attribute. This is unexpected, since the SkyCoord made by get_body does have that information - it is just stored in attributes with a different name, obsgeoloc and obsgeovel (which are CartesianRepresentation instead of EarthLocation for .location). Indeed, if I explicitly set my body.location to the relevant value, body.altaz does work.
So I guess one question is whether we can just translate between these attributes. In principle, the simplest route might be to just let AltAz understand obsgeopos as an alias for location (and maybe eventually deprecate AltAz.location in favour of it).
But perhaps a larger question is why AltAz and, e.g., GCRS are using different attributes in the first place for storing the observer location.
I should add that I'm not 100% sure either is ideal - for general use, I'd want to have the velocity stored on the position as a derivative (i.e., at least obsgeovel as a .derivatives['s'] on obsgeopos -- of course, the derivatives did not exist when obsgeovel was first introduced!). Also, it would seem better to have something that is explicit in what frame the position and velocity are defined. I think the information needed matches exactly what would be an ITRS or GCRS frame instance. So, perhaps one should have a single attribute that replaces location, obsgeoloc and obsgeovel? (and deprecate those three?) Reuse location? New obs_itrs?
cc @StuartLittlefair, @mkbrewer, @eteq, @adrn, @astrojuanlu
from astropy.coordinates import get_body, ITRS
from astropy.time import Time
from astropy import units as u
body = get_body('mars', Time(['J2010'], location=(10.*u.deg, 20*u.deg)))
# The following fails:
body.altaz
AttributeError Traceback (most recent call last)
<ipython-input-4-d1487e3dca10> in <module>
----> 1 body.altaz
~/data/mhvk/packages/astropy/astropy/coordinates/sky_coordinate.py in __getattr__(self, attr)
784 frame_cls = frame_transform_graph.lookup_name(attr)
785 if frame_cls is not None and self.frame.is_transformable_to(frame_cls):
--> 786 return self.transform_to(attr)
787
788 # Fail
~/data/mhvk/packages/astropy/astropy/coordinates/sky_coordinate.py in transform_to(self, frame, merge_attributes)
617 # Do the transformation, returning a coordinate frame of the desired
618 # final type (not generic).
--> 619 new_coord = trans(self.frame, generic_frame)
620
621 # Finally make the new SkyCoord object from the `new_coord` and
~/data/mhvk/packages/astropy/astropy/coordinates/transformations.py in __call__(self, fromcoord, toframe)
1420
1421 curr_toframe = t.tosys(**frattrs)
-> 1422 curr_coord = t(curr_coord, curr_toframe)
1423
1424 # this is safe even in the case where self.transforms is empty, because
~/data/mhvk/packages/astropy/astropy/coordinates/transformations.py in __call__(self, fromcoord, toframe)
1023 return reprwithoutdiff.realize_frame(reprwithdiff)
1024 else:
-> 1025 return supcall(fromcoord, toframe)
1026
1027
~/data/mhvk/packages/astropy/astropy/coordinates/builtin_frames/cirs_observed_transforms.py in cirs_to_altaz(cirs_coo, altaz_frame)
39
40 # first set up the astrometry context for CIRS<->AltAz
---> 41 astrom = erfa_astrom.get().apio(altaz_frame)
42 az, zen, _, _, _ = erfa.atioq(cirs_ra, cirs_dec, astrom)
43
~/data/mhvk/packages/astropy/astropy/coordinates/erfa_astrom.py in apio(frame_or_coord)
109 For this function, an AltAz frame is expected.
110 '''
--> 111 lon, lat, height = frame_or_coord.location.to_geodetic('WGS84')
112 jd1_tt, jd2_tt = get_jd12(frame_or_coord.obstime, 'tt')
113
AttributeError: 'NoneType' object has no attribute 'to_geodetic'
# but this works
body.location = ITRS(body.obsgeoloc).earth_location
body.altaz
<SkyCoord (AltAz: obstime=['J2010.000'], location=[(-2061640.62248365, 5629477.088513, 2169697.89603318)] m, pressure=0.0 hPa, temperature=0.0 deg_C, relative_humidity=0.0, obswl=1.0 micron): (az, alt, distance) in (deg, deg, m)
[(281.60515631, 26.39730811, 1.10516762e+11)]>
Activity
StuartLittlefair commentedon Nov 6, 2020
This has been a particular pain point for me recently, as a result of implementing the TETE frame and geocentric CIRS. My thoughts on it were this:
Generality - specifying an observer location with lat, lon, height is less general. It assumes an Earth-fixed observer. It makes sense for AltAz perhaps, but not for the other frames. I'd be against using
locationas the observer information on all frames.Efficiency - for ease of use with the erfa transformations, there's no preferred way of storing an observer location. Some routines - e.g
apco,apiowant the latitude, longitude and height; others want the observatory's geocentric position and velocity (apcs). At the moment, observer locations are usually coded in the frame in the efficient way - i.e the frame stores in the information in the form needed by the erfa transforms.Consistency - like SkyField, where all vectors are oriented w.r.t BCRS it made some sense to me to store observatory locations on all topocentric frames in the same coordinate system, and GCRS makes sense for almost all use cases. This also means one can easily check if GCRS/CIRS/TETE frames share the same observer. There's an argument to rename these attributes to
gcrs_obslocor similar I guess.There are some disadvantages to this - i.e in #10994 I have to convert
AltAz.locationinto GCRS pos/vel to convert the CIRS coordinate into the correct topocentric CIRS frame, which is slow.Overall, I like the way we store the observer location now, but I can see the merit in the transformation machinery being able to infer the missing attributes if the information is present on the SkyCoord to do so.
mhvk commentedon Nov 6, 2020
@StuartLittlefair - it does seem the
AltAz.locationalways is used as inlocitrs = altaz_frame.location.get_itrs(obstime), i.e., ITRS, but the others assume GCRS. It might actually be easiest if the location is simply a frame instance of any kind, and gets transformed as needed.But more short-term, just hacking in the equivalency inside
SkyCoord.transform_tomay be easiest...StuartLittlefair commentedon Nov 6, 2020
You've just blown my mind. Could that work? Frame instances as attributes of frames?
ayshih commentedon Nov 6, 2020
I'll note that SunPy already makes broad use of
CoordinateAttributefor using frame instances to specify observer locations for our observer-based frames. (Technically, we subclass that frame-attribute class, and in our case, the frame instance is automatically transformed to ourHeliographicStonyhurstframe.)mhvk commentedon Nov 6, 2020
And we also use
CoordinateAttributeinternally inSkyOffsetFrame- I guess one just has to be sure that the frame in the attribute does not refer to its parent (or at least does not need a transformation of its parent that needs itself).StuartLittlefair commentedon Nov 7, 2020
@mkbrewer has suggestions about how to tweak the transforms if we used
locationfor all these frames. See #10994 (comment).This does have some advantages, it's certainly the easiest for the user to understand and perhaps could be efficient. My main concern - which could be a distaster for SunPy - is that it is only useful for a terrestrial observer.
mkbrewer commentedon Nov 7, 2020
Do CIRS, TETE and AltAz have an application for a non-terrestrial observer? It seems to me that earth rotation, precession and nutation wouldn't be applicable there. If a non-terrestrial observer needs to use an Earth centered coordinate system, GCRS would be more appropriate and all that's needed there is to specify the observer's position and velocity wrt the geocenter.
mkbrewer commentedon Nov 7, 2020
That said, GCRS position and velocity can easily be used to construct a pseudo
EarthLocationby using the appropriate transforms, if desired. In fact, a method could easily be added to theEarthLocationclass to do just that.mhvk commentedon Nov 7, 2020
I like the idea for
EarthLocationfor CIRS, TETE, and AltAz, as well as a possibleget_cirs_posvel()extension.Also agree that it would be super useful to think a bit more about observer locations away from Earth, in Earth orbit, at the Earth-Sun Lagrange points, etc. For those having an actual coordinate may end up easiest , i.e., to adapt the approach of SunPy. It may end up being easiest to just allow both approaches for some frames. @ayshih - what is the name you use for this observer coordinate? Could you perhaps point me to the code for a frame that uses it? (Sorry, I know I almost certainly can find this myself, but am hoping it will take you much less time that it would me!)
ayshih commentedon Nov 8, 2020
SunPy calls the frame attribute
observer.To illustrate, here's the documentation for one of our observer-based frames: Heliocentric. In this frame, the Z-axis is aligned with the Sun-observer line. The
observerframe-attribute type is actually subclassed fromCoordinateAttributeto allow for string input that evaluates to a coordinate viaget_body_barycentric().The transformation functions use that observer information as needed. For example, when transforming from our
HeliographicStonyhursttoHeliocentric, it figures out the heliographic longitude/latitude of theobserverfrom theHeliocentricframe, with care taken for any change inobstime, and then calculates the rotation matrix. (code link)StuartLittlefair commentedon Nov 8, 2020
I’m thinking that, for the Earth-bound systems it is simplest to use
locationand GCRS would retain its current behaviour for flexibility.Is there any reason the coordinates of the observer in any given system (eg gcrs) can’t be a lazy property of the frame, which is calculated once and invalidated if the obstime changes on the frame (if that’s even possible)?
mhvk commentedon Nov 8, 2020
@StuartLittlefair - Agreed that that is simplest. In principle, I think lazy properties should be possible; the trick may perhaps be more of how to implement it for the frame transformations - those right now assume that all attributes can also be used to initialize the frame, which would suggest that we need to be able to pass in
location,obsgeolocandobsgeovelto all (and perhaps the can be "secondary attributes" of each other).Given this trickiness, an
observercoordinate as is done in sunpy might actually be easier (where the attribute itself might well be able to take anEarthLocationas input; should check how sunpy does it...). But obviously that means deprecating the others...mkbrewer commentedon Nov 16, 2020
@StuartLittlefair @mhvk It's been over a week here. Does that mean that you are stuck deciding among the various options or just that you are busy with other things?
mhvk commentedon Nov 16, 2020
Definitely busy with other things on my side. I think first decision point is whether for #10994 to pick an
EarthLocationfor CIRS rather thanobsgeo{loc|vel}- but that is best discussed there!astrojuanlu commentedon Nov 23, 2020
At the moment (current
master):CIRS,TETE, andAltAzhavelocationas aEarthLocationAttributeGCRSandPrecessedGeocentrichaveobsgeo{loc,vel}asCartesianRepresentationAttribute.I agree with either (a) adding a new
observer: CoordinateAttributethat deprecateslocationandobsgeo{loc,vel}or (b) reusinglocationasCoordinateAttributeand deprecatingobsgeo{loc,vel}. I guess the implementation and the disruption would depend a lot on whetherEarthLocationshould be an ITRS subclass (#4261), but either way I support unifying the way we store "locations/observers".Cadair commentedon Nov 23, 2020
Just going to chime in and say a +1 to
observerhaving that be consistent with the astropy frames would be awesome.mkbrewer commentedon Nov 24, 2020
On (b), the problem with that is that GCRS is the natural earth centered frame for an observer in earth orbit where the ITRS
EarthLocationframe doesn't apply. This is why we limitedlocationto the ground based CIRS, TETE and AltAz frames.mhvk commentedon Nov 24, 2020
It may still be we need an
ObserverCoordclass that can be any frame, but internally tracks whether it is stuck to the Earth or not (i.e., if it is initialized with anEarthLocation). Although ideally it is extensible to other solar system bodies, in case one wantsAltAzon Mars!mkbrewer commentedon Nov 24, 2020
I think you'd need a
MarsLocationsubclass for that and the theory for that hasn't been invented yet. Not to mention precession and nutation.astrojuanlu commentedon Nov 24, 2020
(For reference frames of other planets, check out #5898)
Cadair commentedon Nov 25, 2020
I have long been thinking about an
ObserverLocationclass which could support moving the observer in time. I.e. you could initialise it with anEarthLocationand then evaluate it in ICRS at any given time. This could then be extended by specifying an orbit propagator for satellites etc. Core could implement this "orbit propagator" for known bodies were ERFA can evaulate the location of say Earth at any time.Basically I see the API as two components, an
ObserverLocationclass and some callable (probably class) which can transform the location to a concrete frame at a given time. You could then imagine subclasses of the callable which use poliastro or SPICE or just ERFA to do the calculations.mhvk commentedon Nov 25, 2020
@Cadair - we have been separately thinking about propagators - that is obviously useful for targets as well! See #10404. Overall, I'm liking the sunpy approach of just using a regular coordinate for the observer more and more - it may just have to be able to propagate knowledge of whether it is stuck to the Earth. (Though perhaps getting a very small velocity in ITRS would be enough.)
Cadair commentedon Nov 25, 2020
sunpy has also done some hacks to retain the name of an observer once it is converted to a real coordinate (via a call to get body), so something along those lines would also be useful.
pllim commentedon Apr 28, 2021
mhvk commentedon Apr 28, 2021