-
Notifications
You must be signed in to change notification settings - Fork 16
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
Pass on ignore_zero_weight to medsreaders. Fill in AM missing pixels #232
base: master
Are you sure you want to change the base?
Conversation
Add a test
The flux is set using a template fitting method, same as used for the psf flux fitter. Zero weight pixels are filled in using the properly normalized model. The adaptive moments output now uses this flux and error rather than the equivalent of a weighted moment flux.
This is no longer the right comparison, since the flux will now be from a template fit
This PR has two parts. The first is to expose the ignore_zero_weight to the medsreaders, so we can create Observations with zero weights or uberseg with these zero weight pixels in the pixels array. This faciliates filling in the pixels in AM Second is to 1) calculate a flux for the current AM model and 2) fill in zero weight pixels using the properly normalized model. I also am now using this flux/flux error and flags in the admom result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the change to Admom filling in pixels should be optional and set when you make the fitter. Otherwise we are pushing a breaking change that cannot be reversed later.
Filing only occurs when there are zero weight pixels, which is not the default for Observations. No examples in the /examples director showed using it. Anyone using admom with masked data in the past was getting a biased result. |
Right but I want to try symmetrized zero-weight pixels. I can't with this PR. |
Yes you can, just construct the Observation with the default |
Ah I see. Thank you. |
This results in an extra evaluation of the model over the pixels during each iteration, so is sub-optimal. I think we can probably eliminate some of the passes by pre-evaluating the model. But I first want to test how this works in some simulations before optimizing. |
this will allow testing using the old cuts future work explore flux err
fc = res['sums_cov'][5, 5] | ||
if fc > 0: | ||
res['s2n'] = res['sums'][5] / np.sqrt(fc) | ||
res['flux_err'] = res['flux'] / res['s2n'] | ||
else: | ||
res['flux_flags'] |= ngmix.flags.NONPOS_VAR | ||
res['flags'] |= ngmix.flags.NONPOS_VAR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be careful here. The old sums[5] entry needs some factors to make its normalization match other things properly.
See this comment I had
# this is a fun set of factors
# jacobian area is because ngmix works in flux units
# the wgt_norm and wsum compute the weighted flux and normalize
# the weight kernel to peak at 1
fnorm = jac_area * wgt_norm * res["wsum"]
res['flux'] = res['sums'][5] / fnorm
I think what you have works but it might be worth leaving some comments for future folks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, this is temporary
No description provided.