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

ENH add photon shooting #82

Merged
merged 95 commits into from
Dec 19, 2023
Merged

ENH add photon shooting #82

merged 95 commits into from
Dec 19, 2023

Conversation

beckermr
Copy link
Collaborator

@beckermr beckermr commented Nov 13, 2023

This PR is another monster. I am happy to break it up for folks if needed. The main reason it got so big is that I wanted to ensure enough tests were running that I flushed out all of the bugs. There were quite a few in fact.

Main features:

  • I added implementations for PhotonArray, Sensors, and associated photon shooting methods on GSObjects
  • I removed test routines copied from galsim in favor of pointing to the galsim test suite with changes as needed. This change will make it easier to sync the code in those routines with the upstream galsim ones as galsim APIs change.
  • I refactored how the random number generators work so that state can be carried everywhere except across JIT boundaries.
  • I added specialized interfaces for photon shooting with fixed-size arrays. This makes those methods JIT compatible. The initial performance tests are better than expected.
  • I refactored a bit of the test suite to ensure nothing was removed since we had some custom tests with the galsim ones we had copied.

To do:

  • add PhotonArrays
  • enable shooting for GSObjects
  • enable tests from galsim
  • add tests of RNG handling to make sure seed and state are propagated correctly
  • add comments to dense code
  • add API tests for PhotonArray
  • add API tests for Sensors
  • add tests of fixed-size photon array and mask handling

jax_galsim/random.py Outdated Show resolved Hide resolved
jax_galsim/random.py Outdated Show resolved Hide resolved
jax_galsim/random.py Outdated Show resolved Hide resolved
jax_galsim/random.py Outdated Show resolved Hide resolved
jax_galsim/random.py Outdated Show resolved Hide resolved
jax_galsim/random.py Outdated Show resolved Hide resolved
jax_galsim/core/utils.py Outdated Show resolved Hide resolved
jax_galsim/core/utils.py Outdated Show resolved Hide resolved
jax_galsim/core/utils.py Outdated Show resolved Hide resolved
@beckermr
Copy link
Collaborator Author

beckermr commented Dec 2, 2023

This PR is ready for review. I expect it to take a while since it is so big. If you'd like it broken up, feel free to suggest ways to do so.

@ismael-mendoza
Copy link
Collaborator

Hi @beckermr - just an update. I'm making good progress with the review and should be finished in the next couple of days

Copy link
Collaborator

@ismael-mendoza ismael-mendoza left a comment

Choose a reason for hiding this comment

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

Hi @beckermr - here is my initial review. Couldn't find any major issues or bugs, although I do have minor suggestions and questions.

tests/galsim_tests_config.yaml Outdated Show resolved Hide resolved
tests/galsim_tests_config.yaml Outdated Show resolved Hide resolved
jax_galsim/image.py Outdated Show resolved Hide resolved
jax_galsim/convolve.py Outdated Show resolved Hide resolved
jax_galsim/core/utils.py Show resolved Hide resolved
jax_galsim/gsobject.py Show resolved Hide resolved
jax_galsim/gsobject.py Outdated Show resolved Hide resolved
jax_galsim/gsobject.py Show resolved Hide resolved
jax_galsim/gsobject.py Show resolved Hide resolved
jax_galsim/core/draw.py Outdated Show resolved Hide resolved
jax_galsim/core/draw.py Outdated Show resolved Hide resolved
jax_galsim/interpolant.py Outdated Show resolved Hide resolved
beckermr and others added 11 commits December 13, 2023 12:34
Co-authored-by: Ismael Mendoza <11745764+ismael-mendoza@users.noreply.github.com>
Co-authored-by: Ismael Mendoza <11745764+ismael-mendoza@users.noreply.github.com>
Co-authored-by: Ismael Mendoza <11745764+ismael-mendoza@users.noreply.github.com>
Co-authored-by: Ismael Mendoza <11745764+ismael-mendoza@users.noreply.github.com>
@beckermr
Copy link
Collaborator Author

OK @ismael-mendoza. Back to you!

Copy link
Collaborator

@ismael-mendoza ismael-mendoza left a comment

Choose a reason for hiding this comment

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

LGTM !

@beckermr beckermr merged commit 7e688ed into GalSim-developers:main Dec 19, 2023
3 checks passed
@beckermr beckermr deleted the photon-array branch December 19, 2023 17:48
@ismael-mendoza ismael-mendoza mentioned this pull request Dec 19, 2023
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.

3 participants