JAX-GalSim icon indicating copy to clipboard operation
JAX-GalSim copied to clipboard

immutability in Image class

Open ismael-mendoza opened this issue 1 year ago • 1 comments

Reposted from #16 from @EiffL

There are some questions about this though. galsim.Image are mutable objects, which really does not play nicely with JAX, it is simply not possible to assign values to array in jax without making a copy. We can follow the galsim API, but the behavior will be different, it won't be possible to assign values to a view of a larger image.

In particular, we cannot do the following in Jax-GalSim:

gal = galsim.Gaussian(sigma=1)
im = galsim.Image(nx=128, ny=128, scale=0.1)

imview = im.view()

gal.drawImage(imview)

This would not update the original im.

This will become even sharper of a question when we go to the next PR, which implements drawing functions.

Already here, there is a question of whether we want to make all Image immutable explicitly, whether we want to allow this sort of things:

image = galsim.Image(nx=128, ny=128, scale=0.1)
image.wcs = galsim.PixelScale(0.2)
image.setCenter(0.0, 0.)

these are examples of bad OO patterns that do not play nicely at all with the JAX functional spirit, but if we remove them, we break the GalSim API....

ismael-mendoza avatar Jun 20 '23 16:06 ismael-mendoza

A couple of methods in the Image class still modify the object, perhaps we should change them so that they return a new object to be more in the JAX spirit?

  • setSubImage
  • setitem
  • resize

ismael-mendoza avatar Jul 20 '23 00:07 ismael-mendoza

I think we solved all of the issues here. The key is that galsim Image objects hold their underlying data as ._array. So we can replace that array underneath and present the OO API to the user. What do you think @ismael-mendoza?

beckermr avatar Sep 16 '24 15:09 beckermr

I think we have resolved this like you say @beckermr so I'm closing this issue

ismael-mendoza avatar Sep 16 '24 19:09 ismael-mendoza