pmelchior / scarlet2

Scarlet, all new and shiny
MIT License
15 stars 4 forks source link

Update multiresolution renderer and test #52

Closed b-remy closed 6 months ago

b-remy commented 6 months ago

This PR updates the resampling renderer strategy by interpolating all the images (observation and PSFs) on the same grid in Fourier space and adds k-wrapping (Bernstein & Gruen 2014) to match galsim resampling. Some refactoring has been done in interpolation.py as well and a test can be run with data soon here.

pmelchior commented 6 months ago

This looks all good. I assume that the test runs well compared to galsim, so the last question is whether we can get rid of the relative path for loading those files. One could request that anyone running test needs to clone the test data repo, but that still doesn't produce an unambiguous file path. Hence my question if we can download files like that from github over the internet by a URL request.

But thinking about this some more, an alternative would be that scarlet-test-repo defines these file paths relative to its installed location, so that one could this e.g. this:

import scarlet_test_data.data_path as data_path
import astropy.io.fits as fits

obs_hdu = fits.open(sdata.resampling.obs_hsc_path)

Or possible even simpler:

from scarlet_test_data import data_path
import astropy.io.fits as fits
import os

obs_hdu = fits.open(os.path.join(data_path, "test_resampling", "Cut_HSC1.fits"))

I like the last option best because it avoids defining a special variable simply to resolve a path where only the base directory is unknown.

The things that are therefore needed:

  1. a __init__.py file for scarlet-test-data that checks the modules installed location, adds "/data" to it, and store that in the variable data_path.
  2. our test scripts to start with a call to pip install git+https://github.com/astro-data-lab/scarlet-test-data.git

For 1, this should do:

data_path = os.path.join(os.path.split(__file__)[0], "data")

I also think we need to define the package name with _ instead of - because the latter cannot be used for imports.

b-remy commented 6 months ago

I see your point and I agree it's much better without hard coded relative data paths. The way I fixed it is starting test files with

from utils import import_scarlet_test_data
import_scarlet_test_data()

which pip installs the data repo if not already installed, and applied your data_path approach. Let me know if this works for you!