Closed rmjarvis closed 1 year ago
Actually, I have one more thing while we're here. I think the following really ought to work, but it doesn't:
sed = galsim.SED(
galsim.LookupTable(
[0, 621, 622, 623, np.inf],
# [0, 621, 622, 623, 10000],
[0, 0, 1, 0, 0],
interpolant='linear'
),
wave_type='nm',
flux_type='fphotons'
)
bp = galsim.Bandpass("LSST_r.dat", 'nm')
print(sed.calculateFlux(bp)) # gives nan
If I swap the np.inf for 10000 though, it does seem to work. Something about Table::integrateProduct
I suspect, though I didn't dive in.
I was looking through the profile Jim made recently of an imSim run, which inspired the
combine_wave_list
speed up in #1243. This PR continues in that vein with various optimizations, mostly related to the SEDs.combine_wave_list
twice whenever we do those calculations. Once internal to the SED object, and once with a separatewave_list
attribute in the chromatic object. But AFAICT, there is no possible way for these two wave lists to get out of sync. The only difference between them is that the one in the ChromaticObject can sometimes have an initial 0 wave and a final np.inf wave. Other than that, they were always identical. Furthermore, those bounding values were never useful for anything. So I switched thewave_list
in the ChromaticObject to be a property that just returnsself.SED.wave_list
. This didn't require any other adjustments in the code or tests, so I'm pretty confident it's functionally equivalent and saves half the calls tocombine_wave_list
.<frozen importlib._bootstrap>:389(parent)
in the profile, and I finally figured out that these are whenimport
statements happen inside a function rather than at the top of a module. I guess it's when python checks to see if it needs to import anything and realizes it already has everything imported. It's not a huge amount of time, but it's wasteful. So I went through the whole code base and moved as many imports as possible out of functions to module scope. Some of these required a little care to avoid circular import patterns. But since it was a lot of lines changed with no real content, I did this directly on main. If anyone wants to look, it's commit 5388b0ec61565897. But I don't think there is anything really substantive there. Anyway, this also led to a non-trivial speed up for faint objects that don't require a lot of work, so tiny bits of overhead are noticeable.obj.SED
. So I decided while I was working on all this to change it to lowercase. (The capital one works still, but it deprecated.) This matches our prevailing style in GalSim to use lowercase attributes, even for abbreviations that are otherwise usually capitalized. (e.g. wcs, psf)The relevant timing script is
devel/time_faint_bd_gals.py
. On my laptop, the time to draw 1000 galaxies went from 1.48 seconds before these changes down to 0.47 seconds after. So a factor of 3 faster, which feels pretty good to me. The remaining tall (ish) poles are initializing the knots in c++, multiplying the sed by a scalar to get the final flux right, making the non-chromatic Transformation object, and initializing the SED objects. Here is the full profile now, fwiw(The
__get__
call in the second line is our lazy_property decorator, so it's just farming out to various other things and then writing the result as an attribute.) Nothing jumps out as particularly egregious now.