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

Add optional higher order moments #244

Merged
merged 8 commits into from
Nov 1, 2024
Merged

Add optional higher order moments #244

merged 8 commits into from
Nov 1, 2024

Conversation

esheldon
Copy link
Owner

@esheldon esheldon commented Nov 1, 2024

These are part of the sums/sums_cov arrays returned by gmix.get_weighted_moments when with_higher_order=True is sent.

This is also available with GaussMom(fwhm, with_higher_order=True)

Currently the sums are calculated and are stored in a larger "sums" and "sums_cov" arrays in the results. No normalized moments are returned.

Names and indices for moments can be found in moments.MOMENTS_NAME_MAP

These are part of the sums/sums_cov arrays returned
by gmix.get_weighted_moments

This is also available with GaussMom(fwhm, higher=True)

Currently the sums are calculated and are stored in a larger "sums" and
"sums_cov" arrays in the results.  No normalized moments are returned.

Names and indices for moments can be found in moments.MOMENTS_NAME_MAP
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.

They keyword higher is not so clear in my opinion. Maybe we can use with_higher_order or similar?

ngmix/gmix/gmix.py Outdated Show resolved Hide resolved
@esheldon
Copy link
Owner Author

esheldon commented Nov 1, 2024

Here's a question: should we just always calculate these higher order moments? They probably don't cost much compared to evaluating the gaussian, and the user will usually copy what they want out

@beckermr
Copy link
Collaborator

beckermr commented Nov 1, 2024

I am concerned other APIs / users expect the shape on output to be 6x6 so if we always calculated them we'd cause API breaks.

@esheldon
Copy link
Owner Author

esheldon commented Nov 1, 2024

Good point

gaussmom use default maxrad from gmix
Big enough that the weight function should be zero.  chose
100 sigma
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.

LGTM from me!

didn't matter since its zero...
@esheldon esheldon merged commit 251a32c into master Nov 1, 2024
4 checks passed
@esheldon esheldon deleted the higher-order-mom branch November 1, 2024 18:27
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.

2 participants