transientskp / pyse

Python Source Extractor
BSD 2-Clause "Simplified" License
11 stars 5 forks source link

Fudge max pix and maximum pixel method variance should only be called once per image #25

Closed HannoSpreeuw closed 3 years ago

HannoSpreeuw commented 3 years ago

This branch was created to bypass repeated calculations of fudge_max_pix, max_pix_variance, beamsize and correlation_lengths, by adding them as an argument to the fit method to the extract.Island objects, instead of repeatedly calculating these same quantities for each island. Also the unit tests have been adapted to the new interfaces to e.g. source_profile_and_errors. I have mixed feelings about the commits in this branch. Yes, I achieved a considerable (~33%) speedup in _pyse. But I could have perhaps achieved the same speedups by using something like the Memoize or functools.cache decorator to e.g. fudge_max_pix. This would have required less of a code overhaul. And calculate_correlation_lenths and calculate_beamsize are lightweight anyway. After this merge it will be somewhat harder to accommodate for a varying psf across the image. This could be fixed by either going back to the situation from before all commits on this branch and adding the functools.cache decorators for compute intense calculations like fudge_max_pix or by making fudge_max_pix_factor, max_pix_variance_factor, beamsize and correlation_lengths attributes to the extract.Island objects, like beam. The latter should actually not be an attribute to the extract.Island objects, in the current philosophy, since it is an attribute to the ImageData object, so not specific for the islands. That would not be the case with a varying psf across the image. However, a varying psf would also have an effect on e.g. kappa, sigma clipping, since the correlation lengths would vary across the image. So accommodating for a varying psf would involve a major code overhaul.