GalSim-developers / GalSim

The modular galaxy image simulation toolkit. Documentation:
http://galsim-developers.github.io/GalSim/
Other
227 stars 107 forks source link

Add _force_stepk to VonKarman and delay building the real-space profile table. #1059

Closed jmeyers314 closed 5 years ago

jmeyers314 commented 5 years ago

Currently galsim.VonKarman builds or retrieves from cache a lookup table for the real-space surface brightness profile upon construction. During table construction, values for the _hlr and _stepk are also computed. When drawing the profile in fft mode, I think the only part of the above that is used is _stepk. The construction of the table is fairly expensive, around ~0.3s on my computer. This can be problematic in a fitting context, where many different values of, say, r0 are attempted in rapid succession.

I propose that we add a constructor argument _force_stepk, and if force_stepk is used, delay building the real-space table until required by a call to either xValue or getHalfLightRadius.

rmjarvis commented 5 years ago

Sounds fine to me. Indeed I think we can follow the pattern used by Sersic and always delay the building of the radial function until it is needed. There's code like if (!_sampler) { ... build _radial and _sampler }. For VK, _radial is also needed for xValue, _hlr and _stepk, so there would be more places to add this and build _radial if it doesn't exist yet, but that's ok.

Could also think about whether we need to be quite so precise for stepk. We might be able to estimate pretty well the maximum R to enclose 1-folding_threshold without actually building up the full radial function. This is also something done by Sersic. This could obviate the need for _force_stepk if the stepk calculation were fast enough. (Although I'm also fine with adding that if you think it is independently useful.)

jmeyers314 commented 5 years ago

The math looks trickier to me in the case of VK since there's no analytic version of the real-space profile to start from.

I think the _force_stepk version could be useful anyway though: The a typical value of stepk I'm seeing in Piff is ~1, which implies a radius of pi arcsec ~ 12 DES pixels. For a PSF stamp size of ~20 pixels then, we're right on the edge of what I'd naively force. For wider PSFs, forcing a stepk of 1 could be a (small) speed improvement. (Although, maybe that means we should use a larger stamp in that case... ¯_(ツ)_/¯ )

rmjarvis commented 4 years ago

cf. #1060