83 add variance image #90
Conversation
|
See linked issue (#83) for proof this works. |
| sci_image.skyrms, templ_image.skyrms, | ||
| data_sci, data_templ, | ||
| cp.zeros((4088, 4088)), cp.zeros((4088, 4088)), # Replace with variance | ||
| cp.array( sci_image.image.noise ), cp.array( templ_image.image.noise ), |
There was a problem hiding this comment.
I have a mild preference for creating these as variables outside the SpaceSFFT_CupyFlow call in parallel with the other. array definitions above. I would put this thus in between the templ_psf = ... and before the wwith fits.open(sci_image.detmask_path ... lines. This would just make the code logically easier to read.
- Create all of our arrays
- Call function
phrosty/photometry.py
Outdated
| bkgfunc = LocalBackground(15, 25, MMMBackground()) | ||
| psfphot = PSFPhotometry(psf, fit_shape, localbkg_estimator=bkgfunc) | ||
| psf_results = psfphot(scienceimage, init_params=init_params) | ||
| psf_results = psfphot(scienceimage, error=error, init_params=init_params) |
There was a problem hiding this comment.
I think having a function named psf_phot and a variable named psfphot is asking for confusion. I would suggest more clearly distinct names.
So really this is a comment on line 98, and about something that was already true before this PR.
There was a problem hiding this comment.
It is not a function and a variable--PSFPhotometry is a class in photutils, and I instantiate it as psfphot, and then call it in the line psf_results = psfphot(....
There was a problem hiding this comment.
I'm talking about the function that this line is in, phrosty.photometry.psf_phot
| return {'exptime': exptime, 'area_eff': area_eff, 'gs_zpt': gs_zpt} | ||
|
|
||
| def get_zpt(self, zptimg, psf, band, stars, ap_r=4, ap_phot_only=False, | ||
| def get_zpt(self, zptimg, err, psf, band, stars, ap_r=4, ap_phot_only=False, |
There was a problem hiding this comment.
I see that err is passed to psf_phot, but I don't think it's actually logically used in get_zpt because there's no SNR cut.
I appreciate that this is pre-existing from before this PR, but given the relevance, I observe that I find the comment bloack at the beginning of this function confusing. It says "we may have to do our own zeropoints" but "we're going to use calibration information we get from elsewhere."?
There was a problem hiding this comment.
Hmm... it is, in psf_phot()--unless you mean a larger restructuring to not include psf_phot() in get_zpt()?
There was a problem hiding this comment.
Yes, but get_zpt doesn't actually used the uncertainties returned by psf_phot.
|
Made some of @wmwv's suggested changes. |
83 add variance image 6f5aecb
No description provided.