desihub / gpu_specter

Scratch work for porting spectroperfectionism extractions to GPUs
BSD 3-Clause "New" or "Revised" License
2 stars 3 forks source link

Use pixpad fraction padding by default #60

Closed dmargala closed 3 years ago

dmargala commented 3 years ago

This PR updates the default values for wavepad and pixpad_frac to be similar to default values used in specter. Note that the wave padding in gpu_specter is specified using a fixed integer value rather than a relative fraction.

I also added a few short docstrings for good karma to balance out the snarky update I made to the test_compare_specter unit test in test_core.py.

It's still not passing the largest deviation < 5% sigma test but it will be interesting to see how the self-consistency plots change @sbailey

> srun -n 1 --cpu-bind=cores pytest py/gpu_specter
============================= test session starts ==============================
platform linux -- Python 3.8.8, pytest-6.2.2, py-1.10.0, pluggy-0.13.1
rootdir: /global/u2/d/dmargala/debug-specter/gpu_specter
plugins: anyio-2.2.0
collected 32 items

py/gpu_specter/test/test_core.py .F..                                    [ 12%]
py/gpu_specter/test/test_extract.py ...........                          [ 46%]
py/gpu_specter/test/test_linalg.py ....                                  [ 59%]
py/gpu_specter/test/test_polynomial.py ...                               [ 68%]
py/gpu_specter/test/test_projection_matrix.py ...                        [ 78%]
py/gpu_specter/test/test_psfcoeff.py ....                                [ 90%]
py/gpu_specter/test/test_spots.py ...                                    [100%]

=================================== FAILURES ===================================
...
        #- require that the largest deviation is within 5% of a sigma
>       self.assertLess(np.max(np.abs(pull)), 0.05)
E       AssertionError: 0.06892567444831484 not less than 0.05

py/gpu_specter/test/test_core.py:179: AssertionError
=========================== short test summary info ============================
FAILED py/gpu_specter/test/test_core.py::TestCore::test_compare_specter - Ass...
=================== 1 failed, 31 passed in 71.18s (0:01:11) ====================
sbailey commented 3 years ago

Apologies for the test confusion; I had misread np.average(np.abs(pull) < pull_threshold) vs. np.average(np.abs(pull)) < pull_threshold, and then made a greater vs. less error of my own in replacing the test. I consolidated the tests to your version, but please double check that I didn't screw it up again.

This change for the wavepad and pixpad frac is definitely a big step in the right direction. Once you reconfirm that I didn't make the tests worse again, I'm happy to merge this.

Comparing wavestep=50 vs. 60 on a full frame extraction, gpu_specter is now more consistent with itself than specter is, except for a few isolated pixels where it is dramatically less consistent.

specter self-consistency: image

gpu_specter self-consistency (this branch): image

gpu_specter self-consistency (current master): image

Let's merge this, and then chase those rare outliers separately.

Using this branch of gpu_specter and the desispec branch in desihub/desispec#993:

rawdir=$reduxdir/preproc/20201215/00068040
expdir=$reduxdir/exposures/20201215/00068040

time srun -N 1 -n 20 desi_extract_spectra --mpi \
    -w 5760.0,7620.0,0.8 --nsubbundles 5 --nwavestep 50 \
    -i $rawdir/preproc-r0-00068040.fits \
    -p $expdir/psf-r0-00068040.fits \
    -o /global/cfs/cdirs/desi/users/$USER/dev/gspec/specter50.fits

time srun -N 1 -n 20 desi_extract_spectra --mpi \
    -w 5760.0,7620.0,0.8 --nsubbundles 5 --nwavestep 60 \
    -i $rawdir/preproc-r0-00068040.fits \
    -p $expdir/psf-r0-00068040.fits \
    -o /global/cfs/cdirs/desi/users/$USER/dev/gspec/specter60.fits

time srun -N 1 -n 20 desi_extract_spectra --mpi --gpu-specter \
    -w 5760.0,7620.0,0.8 --nsubbundles 5 --nwavestep 50 \
    -i $rawdir/preproc-r0-00068040.fits \
    -p $expdir/psf-r0-00068040.fits \
    -o /global/cfs/cdirs/desi/users/$USER/dev/gspec/gpu_specter50.fits

time srun -N 1 -n 20 desi_extract_spectra --mpi --gpu-specter \
    -w 5760.0,7620.0,0.8 --nsubbundles 5 --nwavestep 60 \
    -i $rawdir/preproc-r0-00068040.fits \
    -p $expdir/psf-r0-00068040.fits \
    -o /global/cfs/cdirs/desi/users/$USER/dev/gspec/gpu_specter60.fits

plotting code (cut-and-paste from a notebook):

%pylab inline
import numpy as np
import fitsio
import desispec.io

f1 = desispec.io.read_frame('specter50.fits')
f2 = desispec.io.read_frame('specter60.fits')
g1 = desispec.io.read_frame('gpu_specter50.fits')
g2 = desispec.io.read_frame('gpu_specter60.fits')

def plot_compare(s1, s2, plot_title=None):
    err = np.sqrt(1/s1.ivar + 1/s2.ivar)
    dflux = s1.flux - s2.flux
    pull = dflux/err
    medabspull = np.median(np.abs(pull), axis=0)
    maxabspull = np.max(np.abs(pull), axis=0)

    figure(figsize=(12,4))
    subplot(211)
    imshow(pull, aspect='auto', vmin=-0.1, vmax=0.1, cmap='RdBu')
    ylabel('fiber number')
    colorbar()
    if plot_title:
        title(plot_title)
    subplot(212)
    plot(maxabspull, label='max(abspull)')
    plot(medabspull, label='median(abspull)')
    xlim(0, len(maxabspull))
    ylim(-0.01, 0.5)
    legend(loc='upper right')
    xlabel('CCD row')
    ylabel('max or med(|pull|)')
    colorbar()

plot_compare(f1, f2, 'specter nwavestep 50 vs. 60')
plot_compare(g1, g2, 'gpu_specter nwavestep 50 vs. 60')
dmargala commented 3 years ago

That looks much better, awesome! I confirm that the consolidated test is good.