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

#1229 Fix a few errors in photon shooting chromatic transformations. #1236

Merged
merged 3 commits into from
Aug 23, 2023

Conversation

rmjarvis
Copy link
Member

Josh pointed out in #1229 that chromatic objects with wavelength-dependent transformations don't work with photon shooting. This fixes that.

I also fixed two other issues in this part of the code:

  • GalSim automatically adds a WavelengthSampler to the photon_ops for photon shooting chromatic objects. But if the user already did that, then it (1) redundantly samples the wavelengths twice, which is inefficient and (2) warns the user that this is likely an error. Well, IMO, the error is in the GalSim code, not the user code. So now it skips adding a redundant WavelengthSampler and consequently doesn't emit the warning.
  • I realized when writing the test that the handling of the poisson_flux option in ChromaticConvolution was incorrect. It neglected to apply the g factor returned by _calculate_nphotons, which is required to get the rendered poisson flux correct. (Basically because the way we implement this, shooting always tries to get the right total flux with whatever n_photons is, but then multiplying by g gets them back to the poisson value.)

@rmjarvis rmjarvis added this to the v2.5 milestone Aug 15, 2023
@rmjarvis rmjarvis added the chromatic Related to the Chromatic classes, SEDs, bandpasses, etc. label Aug 22, 2023
Copy link
Member

@jmeyers314 jmeyers314 left a comment

Choose a reason for hiding this comment

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

LGTM

@rmjarvis rmjarvis merged commit 3aecaf4 into main Aug 23, 2023
9 checks passed
@rmjarvis rmjarvis deleted the #1229 branch August 23, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chromatic Related to the Chromatic classes, SEDs, bandpasses, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants