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

Pass on ignore_zero_weight to medsreaders. Fill in AM missing pixels #232

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

esheldon
Copy link
Owner

No description provided.

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
@esheldon esheldon requested a review from beckermr November 14, 2022 19:37
@esheldon
Copy link
Owner Author

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

Copy link
Collaborator

@beckermr beckermr left a 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.

CHANGES.md Outdated Show resolved Hide resolved
@esheldon
Copy link
Owner Author

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.

@beckermr
Copy link
Collaborator

Right but I want to try symmetrized zero-weight pixels. I can't with this PR.

@esheldon
Copy link
Owner Author

Yes you can, just construct the Observation with the default ignore_zero_weight=True

@beckermr
Copy link
Collaborator

Ah I see. Thank you.

CHANGES.md Show resolved Hide resolved
@esheldon
Copy link
Owner Author

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.

Comment on lines 418 to 424
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
Copy link
Collaborator

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.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, this is temporary

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

Successfully merging this pull request may close these issues.

None yet

2 participants