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.
| xim2 = kim.calculate_inverse_fft() | ||
| np.testing.assert_almost_equal(xim.array, xim2.array) | ||
| if is_jax_galsim(): | ||
| if dt not in [np.complex128, complex]: |
There was a problem hiding this comment.
could you explain why you skip this test for jax galsim with these datatypes? do we not support them for some reason?
There was a problem hiding this comment.
I had missed this note in the documentation
JAX-GalSim does not support forward FFTs of complex dtypes.
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
3 participants
This PR exists to track the changes we've made for JAX-GalSim testing.
xref: GalSim-developers#1252