GalSim-developers / JAX-GalSim

JAX port of GalSim, for parallelized, GPU accelerated, and differentiable galaxy image simulations.
Other
25 stars 3 forks source link

Adding minimal GSObject, Sum, Gaussian, Exponentional #7

Closed EiffL closed 2 years ago

EiffL commented 2 years ago

This PR aims to add a minimimal working version of a GSObject and a simple light profile, mainly as a demonstrator and proof of concept.

Here is a notebook to try it out: colab link

Functionaliies covered in this PR:

Functionalities that are out of scope for this PR:

EiffL commented 2 years ago

within this PR, all implemented functions should match their reference counterpart. A normal galsim code can be turned into jax-galsim like so:

import jax_galsim as galsim

Pretty cool :-)

See for instance the Exponential tests:

import numpy as np
import jax_galsim as galsim

def test_exponential_properties():
    """Test some basic properties of the Exponential profile.
    """
    test_flux = 17.9
    test_scale = 1.8
    expon = galsim.Exponential(flux=test_flux, scale_radius=test_scale)

    # Check Fourier properties
    np.testing.assert_almost_equal(expon.maxk, 10 / test_scale)
    np.testing.assert_almost_equal(expon.stepk, 0.37436747851 / test_scale)
    np.testing.assert_almost_equal(expon.flux, test_flux)
    # Check input flux vs output flux
    for inFlux in np.logspace(-2, 2, 10):
        expon = galsim.Exponential(flux=inFlux, scale_radius=1.8)
        outFlux = expon.flux
        np.testing.assert_almost_equal(outFlux, inFlux)
ismael-mendoza commented 2 years ago

Awesome! I will take a look early tomorrow morning :) (sorry, just saw the request for review)

EiffL commented 2 years ago

To answer your question, I copy/pasted these tests from GalSim, but I think we need some sort of automated way of reusing existing tests, that I'm still thinking about....

EiffL commented 2 years ago

Reading through your review, I think we need two things:

EiffL commented 2 years ago

oh and also, we need a way to know which GalSim version we are targeting. There is currently quite a bit of difference between the last release and the master branch, although these differences are unlikely to impact basic core functionalities.

rmandelb commented 2 years ago

Do you also need an automated way of checking when GalSim has a new release in which the tests have been updated compared to whatever release you are currently using as a baseline? Or is that something you'd do manually when there is a new release, since they are not super frequent?

EiffL commented 2 years ago

I was thinking concretely of having a git submodule in the test folder, targetting a specific version of galsim, and that this would get manually updated once a new release of galsim is available. Then... to keep track of implementation changes between galsim versions... one hope is that if there are changes in the way galsim behaves from version to version, these would be captured by tests, which would then fail if changes are not made to the jax version.

ismael-mendoza commented 2 years ago

a mechanism/convention to state what is purely copy/pasted from GalSim, what had to handcrafted a mechanism to grab tests from the GalSim repo without needing to copy/paste them. Because at some point we wont remember what test was included what test wasnt

Yup, that would be very nice actually

EiffL commented 2 years ago

Ok, I've proposed a solution for the tests in #14 that we can first try to merge in this branch, so that it benefits from these automated tests.

EiffL commented 2 years ago

I think all good for final review @b-remy @ismael-mendoza, let me know if you see anything else that should be done before merging.

I've added github actions to run the test suite automatically and added a file to test that objects are being jitted correctly by JAX, which is the most likely thing to fail silently in the jax implementation