Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Piff catastrophically fails when PixelGrid size is larger than input star images #131

Open
parejkoj opened this issue Apr 27, 2022 · 3 comments

Comments

@parejkoj
Copy link

We are using Piff in Rubin processing, and have discovered that the model fit for a size=25 PixelGrid catastrophically fails, without any clear indication of the error from Piff itself.

I have attached a stand-alone demonstration of this failure, with a pickle file containing the Piff.Star objects we made in our Rubin test. Extract the tarball and run python piff-fails-size25.py to get a DEBUG-level log output and a matplotlib image showing the incorrect fitted image (we picked location 10,10, but it is bad everywhere, as far as we can tell). Change the 'size': 25 under the note in the python script to 21 or 23 to see a successful result.

At minimum, some indication from Piff that the fit has gone haywire would be extremely useful: we don't see any warning- or error-level messages that are relevant in the output. There is a WARNING:PIFF: Total chisq = 327.64 / 5256 dof message at the end, but I don't think that should be a WARNING at all, because that looks like a reasonable chi2 value, and it's roughly the same chi2 for size=21.

piff-fails.tar.gz

@jmeyers314
Copy link
Collaborator

Err. Just noticed that the Piff.Star objects we attached here are exactly 21x21 pixel stamps. So it's not unreasonable for Piff to fail to extrapolate to 25x25 (though maybe that'd also be a reasonable thing for Piff to warn about). We should try again with larger input stamps.

@parejkoj parejkoj changed the title Piff fails for PixelGrid size=25 Piff catastrophically fails when PixelGrid size is larger than input star images Apr 27, 2022
@parejkoj
Copy link
Author

Thank you for noticing that, @jmeyers314 ! Making sure our psf candidate sources are as large as the model size seems to have done the trick, and I'm implementing checks in our code to try to prevent this in the future.

I've updated the issue title here: PIFF should error out if the configured PixelGrid is larger than the star images that are given as input to the fitter. That would have helped us catch this earlier.

@rmjarvis
Copy link
Owner

It's hard to anticipate all the ways users can fail to use code appropriately. :) But yeah, I agree this is something that Piff should check for and give an appropriate error earlier in the process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants