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

Implement photometry error #73

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Implement photometry error #73

wants to merge 5 commits into from

Conversation

aguinot
Copy link
Collaborator

@aguinot aguinot commented Aug 14, 2024

Solve issue #69
Add photometry error as a possible output for all Euclid bands. It uses gaussian mixtures to approximate galaxy model (with ngmix). The PSF is modeled by a gaussian (not the best but not the worst.)

NOTE: merge after #72

@aguinot aguinot added this to the near-term small items milestone Aug 14, 2024
@aguinot aguinot added the enhancement New feature or request label Aug 14, 2024
@rmandelb rmandelb self-requested a review August 14, 2024 14:29
Copy link
Member

@rmandelb rmandelb left a comment

Choose a reason for hiding this comment

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

I was a bit concerned about unit test coverage and docstrings - we can discuss if you want

@@ -115,6 +115,12 @@
vis_bands = ['VIS']
nisp_bands = ['NISP_Y', 'NISP_J', 'NISP_H']

# Add some information related to NISP
nisp_gain = 2 # https://arxiv.org/pdf/2405.13496 Sect 4.3.7
nisp_pixel_scale = 0.3
Copy link
Member

Choose a reason for hiding this comment

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

give units

@nb.njit
def circular_aper(N, r):
"""
Draws a circle of radius r in a square image of size N x N.
Copy link
Member

Choose a reason for hiding this comment

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

Say units for r

MAX_IMG_SIZE = 501


def get_good_img_size(gmix, scale):
Copy link
Member

Choose a reason for hiding this comment

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

It would be helpful for future developers if you could add docstrings for some of these functions, even just simple ones for those that are not user-facing

@aguinot aguinot mentioned this pull request Sep 25, 2024
30 tasks
@rmandelb
Copy link
Member

Looks like there are open review comments and the branch is behind main. What is the plan for this one, @aguinot ?

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

Successfully merging this pull request may close these issues.

2 participants