Skip to content

[DO NOT MERGE] changes for JAX-GalSim testing#1

Open
beckermr wants to merge 385 commits intomainfrom
fix_tests
Open

[DO NOT MERGE] changes for JAX-GalSim testing#1
beckermr wants to merge 385 commits intomainfrom
fix_tests

Conversation

@beckermr
Copy link
Copy Markdown
Collaborator

@beckermr beckermr commented Mar 25, 2026

This PR exists to track the changes we've made for JAX-GalSim testing.

xref: GalSim-developers#1252

Copy link
Copy Markdown

@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.

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
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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?)

Comment thread tests/test_moffat.py Outdated
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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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?

Comment on lines +378 to +379
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)
Copy link
Copy Markdown

@ismael-mendoza ismael-mendoza Apr 2, 2026

Choose a reason for hiding this comment

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

is this loss accuracy for the interpolants at the level that we should improve/care/document?

Comment thread tests/test_draw.py
Comment on lines +530 to +538
if is_jax_galsim():
maxk_threshold = 1.e-3
N = 880
Ns = 28
else:
maxk_threshold = 1.e-4
N = 1174
Ns = 37

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

could you explain this modification? (speed?)

Comment thread tests/test_draw.py
@@ -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
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

could you explain why?

Comment thread tests/test_draw.py
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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

what do you mean exactly? not sure I follow why this happens.

Comment thread tests/test_draw.py
Comment thread tests/test_moffat.py
Comment on lines +156 to +157
if is_jax_galsim():
np.testing.assert_allclose(psf.maxk, 11.634597424960159, atol=0, rtol=0.2)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

maybe you are still working on this... but I noticed this branch will never be executed, same for the one below

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