You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently the import speed of the cosmology package is slow. This matters if you want to import cosmology in a startup script for ipython, for example.
I assume this is because there is a lot of code in cosmology/__init__.py. This can probably be streamlined or deferred to make import times much faster.
Have you confirmed that it's the cosmology package in particular that's slow on import and not, say, just astropy itself? Lots of stuff happens in astropy/__init__.py for example.
Is it just that initializing the built-in cosmology objects is slow? ISTM Cosmology initiation could be sped up by lazy computing some of the internal values.
I just took a look at the code, and my comment in the initial post is wrong - there is hardly any code at all in cosmology/__init__.py. I must be remembering wrongly.
This means that the problem is likely due to initialising the built-in cosmology objects, as @embray suggests.
Which happens in cosmology.core which is imported by cosmology.__init__ so close enough. I would strongly suspect that's it. I definitely thing it could be sped up by using lazy evaluation of the internal attributes set up by Cosmology.__init__.
@embray -- do you mean the attributes internal to each cosmology object, or to the cosmology package? The stuff that takes all the time is the initialization of the provided built in cosmologies (WMAP9, Planck13, etc.). And those aren't internal, so being lazy about creating them would take some thought. On the other hand -- the number of built in cosmologies probably only increases with time, so it would be nice to preemptively fight that bloat.
Incidentally, profiling says that the most expensive part of initializing a cosmology object are all unit related. In particular, these lines are quite expensive:
We could certainly speed up the first two -- the definitions of Mpc, km, and s are fixed. For the latter it would be nice to keep the use of the constant library in case a better G measurement comes along. But we could precompute some of the factors for the module rather than on a per-object basis.
I just tried it, and the initialization time for a cosmology object dropped from 300ms to 3ms. Yeah... I should turn that into a pull no matter what @embray meant about laziness!
More detail: it turns out that calling const.G.cgs is very, very expensive (140ms), accounting for almost half the initialization cost. Perhaps somebody should look into that! Without the .cgs it is blazingly fast (81ns). For now I'll just hardwire the factor of 1000, which assumes that constants is going to stay mks by default.
Yeah -- the unit compose method is very slow (called by cgs under the hood). That's a really tricky one to optimize, since there are so many side cases to take into account. However, just caching the result of cgs cut the import time by about a third for me, since the same cgs is computed again and again. I'll have a PR for that shortly.
I think I'll still want to improve the performance of units independent of this (which is what my PR does -- or hopefully will do if I can get it to work). I don't think it hurts to be optimized on both levels.
I was referring to creation of some of the internal attributes of cosmology objects which are slow. Many of them could just as well be @lazypropertys or something.
Activity
embray commentedon May 14, 2014
Have you confirmed that it's the cosmology package in particular that's slow on import and not, say, just astropy itself? Lots of stuff happens in
astropy/__init__.pyfor example.Is it just that initializing the built-in cosmology objects is slow? ISTM
Cosmologyinitiation could be sped up by lazy computing some of the internal values.nhmc commentedon May 15, 2014
Importing astropy is pretty slow, but astropy.cosmology is noticeably slower:
Incidentally if anyone knows a better way to measure import speeds, please let me know.
nhmc commentedon May 15, 2014
I just took a look at the code, and my comment in the initial post is wrong - there is hardly any code at all in
cosmology/__init__.py. I must be remembering wrongly.This means that the problem is likely due to initialising the built-in cosmology objects, as @embray suggests.
embray commentedon May 15, 2014
Which happens in
cosmology.corewhich is imported bycosmology.__init__so close enough. I would strongly suspect that's it. I definitely thing it could be sped up by using lazy evaluation of the internal attributes set up byCosmology.__init__.aconley commentedon May 21, 2014
@embray -- do you mean the attributes internal to each cosmology object, or to the cosmology package? The stuff that takes all the time is the initialization of the provided built in cosmologies (WMAP9, Planck13, etc.). And those aren't internal, so being lazy about creating them would take some thought. On the other hand -- the number of built in cosmologies probably only increases with time, so it would be nice to preemptively fight that bloat.
Incidentally, profiling says that the most expensive part of initializing a cosmology object are all unit related. In particular, these lines are quite expensive:
We could certainly speed up the first two -- the definitions of Mpc, km, and s are fixed. For the latter it would be nice to keep the use of the constant library in case a better G measurement comes along. But we could precompute some of the factors for the module rather than on a per-object basis.
I just tried it, and the initialization time for a cosmology object dropped from 300ms to 3ms. Yeah... I should turn that into a pull no matter what @embray meant about laziness!
aconley commentedon May 21, 2014
More detail: it turns out that calling const.G.cgs is very, very expensive (140ms), accounting for almost half the initialization cost. Perhaps somebody should look into that! Without the .cgs it is blazingly fast (81ns). For now I'll just hardwire the factor of 1000, which assumes that constants is going to stay mks by default.
mdboom commentedon May 21, 2014
Yeah -- the unit
composemethod is very slow (called bycgsunder the hood). That's a really tricky one to optimize, since there are so many side cases to take into account. However, just caching the result ofcgscut the import time by about a third for me, since the same cgs is computed again and again. I'll have a PR for that shortly.aconley commentedon May 21, 2014
Ah, no, don't PR. I'm in the middle of one.
Speedup of initialization for cosmology objects
mdboom commentedon May 21, 2014
I think I'll still want to improve the performance of units independent of this (which is what my PR does -- or hopefully will do if I can get it to work). I don't think it hurts to be optimized on both levels.
aconley commentedon May 21, 2014
Agreed. So... what do I do about the comment in #2542 that it closes this issue? Ignore it and just have you reopen (presuming #2542 is accepted)?
mdboom commentedon May 21, 2014
That's fine. I think you can also edit the description to change the wording and avoid the automatic closing.
embray commentedon May 22, 2014
I was referring to creation of some of the internal attributes of cosmology objects which are slow. Many of them could just as well be
@lazypropertys or something.aconley commentedon May 22, 2014
Ok, that would be a lot easier. But something like 98% of the init time per object has now been removed, so it's probably not as interesting.
Speedup of initialization for cosmology objects