Conversation
|
The commits here need to be rebased onto main I think. |
|
ok @rmjarvis This giant PR is ready for review. I can break it up into smaller PRs if that would help. My hope is that with this PR, we can run jax-galsim tests against the main branch going forward. |
| else: | ||
| check_dep(galsim.GSParams, allowed_flux_variation=0.90) | ||
| check_dep(galsim.GSParams, range_division_for_extrema=50) | ||
| check_dep(galsim.GSParams, small_fraction_of_flux=1.e-6) |
There was a problem hiding this comment.
Honestly, I think there is no reason that JAX-GalSim needs to comport with our now-deprecated behavior. I'd be happy to just have:
if is_jax_galsim(): return
at the start of every test in this file.
| assert_raises(ValueError, galsim.BoundsD, 11, 23, 17, "blue") | ||
| if is_jax_galsim(): | ||
| # jax doesn't raise for this | ||
| pass |
There was a problem hiding this comment.
I think I can guess what it does with floats. But I'm quite curious what it does with "blue".
|
|
||
| if is_jax_galsim(): | ||
| # jax doesn't raise for these things | ||
| pass |
There was a problem hiding this comment.
What does it do for an undefined Bounds if you ask for the center?
| assert wcs == wcs2, name+' local() is not == the original' | ||
| new_origin = galsim.PositionI(123,321) | ||
| wcs3 = wcs.withOrigin(new_origin) | ||
| wcs3 = wcs.shiftOrigin(new_origin) |
There was a problem hiding this comment.
For a LocalWCS, these are equivalent. And IMO withOrigin is preferred, since the WCS doesn't have an origin yet.
This PR has two main features:
It has a generalization of the concept of a "galsim backend" with associated functions for the test suite only. In the test suite only, the backend is used to adjust the tests as needed for the jax-based galsim implementation.
It has adjustments of the tests for jax-galsim. Most of them are related to cases where jax cannot raise the same errors (e.g., for checking argument types) or when the fact that jax arrays cannot be changed in place causes the APIs to differ.