Repository navigation
Re-work / refactor of SkyCoord initializer #7030
Description
This has come up in several contexts, and is an important goal for improving coordinates performance. The SkyCoord initializer has evolved "organically" since it's initial creation in v0.4, which arguably is a euphemism for "not well-planned". @adrn and @mhvk and I have discussed a tentative plan for how to implement this with basically no direct impact to the end-user beyond (hopefully) significant performance improvements.
Also involved in this might be a way to tell SkyCoord "don't do so much checking" (see #7029 and #6941). Philisophically this might seem a bit odd because one might think the initializer is the place for that, and classmethods should be used for cleverer parsing. But we're pretty much stuck with the current API, so that's basically a non-starter. Instead we hope we can do some clever ordering of the parsing so that simple/basic use cases are parsed quickly, bypassing other checks. That lets us have our 🍰 and eat it too. But we shall see once we actually dive in...
Note that this is related to but not quite the same as #7029. Also some related discussion in #6896, #6941, and #5952.
Activity
astrofrog commentedon Jul 1, 2018
While doing #7615, I found that one thing that contributes to poor performance is the fact that the frames often seem to needlessly recompute things. In particular, as far as I understand the following should be specific to a frame class and could in principle be cached at a class level:
representation_inforepresentation_component_namesget_representation_component_namesrepresentation_component_unitsget_representation_component_unitsI say 'in principle' because if one actually tries to cache them at a class level, there are currently issues (see e.g. #7614), because in some cases new classes are created dynamically from old ones and we need to be careful to reset the cache in those situations. I think it's possible, but requires someone who understands more about the infrastructure in astropy.coordinates.
The methods/properties above typically take ~100µs or less to run so I don't think that computing them at class definition time would actually result in a big burden in terms of import time for
astropy.coordinates, and I think there is a factor 2-3 in performance to be gained from doing this.Anyway, this is just to say that part of speeding up the initializer is going to have to be also speeding up access to frame attributes that should in principle never change.
eteq commentedon Oct 18, 2018
I did some profiling on this in https://gist.github.com/b9a4ca6ce17748a45e45a040ce10304e and I find a similar result to @astrofrog.
adrn commentedon Dec 3, 2018
Note: some of this was done for 3.1. We need to revisit and see what specific things should be addressed going forward.
[-]Re-work of `SkyCoord` initializer[/-][+]Re-work / refactor of `SkyCoord` initializer[/+]