Added functions need by optatmo to the util.py about piff HOT 8 CLOSED

def measure_snr(star):

There is already a function InputFiles.calculateSNR. And I believe the result is stored in the props dict of the star. So is this really needed? If so, let's at least move the existing function to util.py and refactor it as needed to make it usable in both contexts.

def estimate_cov_from_jac(jac):

This is part of PR #94.

def calculate_moments(star, third_order=False, fourth_order=False, radial=False, errors=False, logger=None):

This function would be a worthwhile PR on its own, but I think it requires some work.

I worked on this back in August, but had trouble making them really robust. In particular, there are a number of various fudge factors in there, which are not well motivated. And the errors=True option I think is not particularly accurate.

So if someone can really hammer out a solid set of unit tests for this using a wide array of input galaxy profiles, showing that the code is accurate across a realistic swath of inputs, then I'd be happy to get this merged.

from piff.

from piff.

I did test these with some star shapes - assume you meant stars not galaxies here.

Yes. Sorry. I meant a variety of realistic star profiles. :)

from piff.

So, the InputFiles.calculateSNR() is needed b/c in a couple cases the SNR calculate is being simulated rather than stars being read from input files. Since it is static I can call it from where ever at the cost of an added dependence on piff.input.InputFiles. So, it might make sense to pull it to util.py, but that isn't strictly needed. I can confirm that using the function InputFiles.calculateSNR works fine.

from piff.

Similarly estimate_cov_from_jac works fine. So I think once you decide if we want to move calculateSNR to utils or not we can close this issue and open an new one for the hsm moments.

from piff.

If calculateSNR is used from multiple places, I think it makes sense to put it in util. Feel free to do that now if you like. Especially since it's a staticmethod, so there's not any reason really to have it live in InputFiles.

from piff.

Ok, I made a PR for that. Once that gets approved lets close this issue and make a new one for the calculate_moments and associated tests.

from piff.

Ok, the PR has been merged. I'll discuss with Aaron what me need to include the moments.

from piff.