Conversation
There was a problem hiding this comment.
Hey @beckermr these are all the changes I could find to the galsim tests related to tolerances/precision that I thought could be relevant (re this issue). Does that make sense to you or do you think I'm missing any other important ones? We can discuss in our next meeting what we should do about these...
| @@ -299,6 +375,9 @@ def do_shoot(prof, img, name): | |||
| print('nphot = ',nphot) | |||
| img2 = img.copy() | |||
|
|
|||
| if is_jax_galsim(): | |||
| rtol *= 3 | |||
There was a problem hiding this comment.
Could you explain why this change was required to pass the tests? (side note, should we worry about photon shooting tests and such for the paper?)
| np.testing.assert_almost_equal(psf.flux, test_flux) | ||
| np.testing.assert_almost_equal(psf.xValue(cen), psf.max_sb) | ||
| if is_jax_galsim(): | ||
| np.testing.assert_allclose(psf.maxk, 11.634597424960159, atol=0, rtol=0.2) |
There was a problem hiding this comment.
Should we add some sort of caveat in the lax documentation regarding this? Is this what you are trying to improve in GalSim-developers/JAX-GalSim#205?
| np.testing.assert_allclose(ln.kval(x), true_kval, rtol=3.0e-4, atol=3.0e-6) | ||
| np.testing.assert_allclose(ln.kval(x[12]), true_kval[12], rtol=3.0e-4, atol=3.0e-6) |
There was a problem hiding this comment.
is this loss accuracy for the interpolants at the level that we should improve/care/document?
| if is_jax_galsim(): | ||
| maxk_threshold = 1.e-3 | ||
| N = 880 | ||
| Ns = 28 | ||
| else: | ||
| maxk_threshold = 1.e-4 | ||
| N = 1174 | ||
| Ns = 37 | ||
|
|
There was a problem hiding this comment.
could you explain this modification? (speed?)
| @@ -1097,19 +1113,31 @@ def test_shoot(): | |||
| # in exact arithmetic. We had an assert there which blew up in a not very nice way. | |||
| obj = galsim.Gaussian(sigma=0.2398318) + 0.1*galsim.Gaussian(sigma=0.47966352) | |||
| obj = obj.withFlux(100001) | |||
| image1 = galsim.ImageF(32,32, init_value=100) | |||
| if is_jax_galsim(): | |||
| # jax galsim needs double images here | |||
| np.testing.assert_almost_equal(image2.array, image1.array, decimal=12) | ||
| if is_jax_galsim(): | ||
| # jax galsim works not as well | ||
| np.testing.assert_array_almost_equal(image2.array, image1.array, decimal=10) |
There was a problem hiding this comment.
what do you mean exactly? not sure I follow why this happens.
Removed specific OS and compiler combinations from CI configuration to simplify testing.
test: update test_ne to run more tests and to pass more easily for flux
Co-authored-by: Matthew R. Becker <beckermr@users.noreply.github.com>
Co-authored-by: Matthew R. Becker <beckermr@users.noreply.github.com>
test: merge main into tests
test: changes to run more of moffat tests
| if is_jax_galsim(): | ||
| np.testing.assert_allclose(psf.maxk, 11.634597424960159, atol=0, rtol=0.2) |
There was a problem hiding this comment.
maybe you are still working on this... but I noticed this branch will never be executed, same for the one below
test: merge latest changes from main into JAX tests
This PR exists to track the changes we've made for JAX-GalSim testing.
xref: GalSim-developers#1252