U/danielsf/rmjarvis/cut gsco

[danielsf]

This adds a unit test on top of @rmjarvis 's PR storing floats in a numpy array in GalSimCelestialObject

[rmjarvis]

npoints is an int. Probably should either (a) leave that out of _float_values and keep it as a regular attribute or (b) explicitly cast back to an int here in the getter.

Double precision should be fine to capture any int value we would use without weird rounding errors, so (b) is probably fine. But it might cause problems if it's allowed to turn into a float.

[danielsf]

It looks like Francois already casts it into an int when it gets used

https://github.com/lsst/sims_GalSimInterface/blob/master/python/lsst/sims/GalSimInterface/galSimInterpreter.py#L465

I'm not sure how zealous we want to be in cutting every possible bit of python overhead from our memory footprint. If you still think it would be safer to go back to storing self._npoints as an int, I can do that.

[rmjarvis]

It might be slightly paranoid of me, but if you want to be absolutely sure that we're safe, I'd make the getter here return int(self._float_values[15] + 0.5). Just to make sure some floating point imprecision doesn't convert an int into something like 3.3999999999987e1 and get messed up by the (implicit) floor in the conversion back to int.

[rmjarvis]

But again, I do think that we would be safe even without that. I'm just not 100% positive. But rounding will definitely be safe over the range of npoints values we'll ever be using.

[danielsf]

I have implemented your suggestion.

[danielsf]

I will run this through Jenkins. If it passes (and no one objects) I will merge this at the end of the day.

[jchiang87]

I ran my little test of the memory profile just for the GSCO creation and added the profile curve I get using this code:

For the fixed and mj_gsco cases, the memory devoted to the GSCOs is the vertical extent of the corresponding curve starting from the base of that double peaked feature to the maximum (i.e., that linear ramp is the loop over the objects to make the GSCOs). Using the original GSCO class, the uss memory is 0.77GB while using Mike's new version it's 0.50GB--a ~35% savings. BTW, I accidentally overwrote the data for the baseline curve, so I reran it, and it looks a little different from the original figure in my talk yesterday. The relative slowdown before the double peaked feature is just from variation in disk access speed as that part of the curve is reading in the data.

[jchiang87]

lgtm

[cwwalter]

Great... confused by one thing though: are the differences in run time from other speed changes? Or is this really only the GSCO change?

[jchiang87]

The only runtime variation that's apparent to me is the disk access part. The linear ramp for the fixed and mj_gsco are the same since they are processing the same number of objects. The corresponding ramp in the baseline case is longer because it is processing almost twice as many objects as the other two cases.

[cwwalter]

OK that explains it. Thanks.

[rmjarvis]

The corresponding ramp in the baseline case is longer because it is processing almost twice as many objects as the other two cases.

And that is not an input catalog difference -- that's because of your reordering, right? I think you said it used to process a bunch of things that ended up getting cut out at the end, so were actually wasted. So that's a real speedup.