sdss / mangadap

The MaNGA Data Analysis Pipeline
BSD 3-Clause "New" or "Revised" License
16 stars 8 forks source link

Exception raised when using covariance calibration for S/N #115

Closed iandroberts closed 4 months ago

iandroberts commented 5 months ago

I am attempting to run (largely with success) the MaNGA DAP on a cube from the WEAVE Spectrograph Large IFU. However, I am running into an exception when I try to use a covariance calibration to calculate the SN for the Voronoi binning. Specifically, in my configuration file I am setting the following in order to make use of the 1 + alpha * log(Nbin) type calibration

[binning.spatial]
covar = 1.36

This raised the following warning

Binning algorithm has raised an exception.  Assume this is because '
                          'all the spaxels should be in the same bin.

The stacked S/N for my cube is comfortably above my Voronoi target S/N, so I didn't think that everything should be in a single bin (indeed I have single pixels that are above my Vornoi target S/N).

I dug a little deeper and the specific error that is being raised by the voronoi_2d_binning function is the following

Traceback (most recent call last):
  File "/home/ian/miniconda3/envs/dap/bin/manga_dap", line 8, in <module>
    sys.exit(MangaDap.entry_point())
  File "/home/ian/miniconda3/envs/dap/lib/python3.9/site-packages/mangadap/scripts/scriptbase.py", line 114, in entry_point
    cls.main(cls.parse_args())
  File "/home/ian/miniconda3/envs/dap/lib/python3.9/site-packages/mangadap/scripts/manga_dap.py", line 102, in main
    status = manga_dap(cube, plan, dbg=args.dbg, log=args.log, verbose=args.verbose)
  File "/home/ian/miniconda3/envs/dap/lib/python3.9/site-packages/mangadap/survey/manga_dap.py", line 171, in manga_dap
    SpatiallyBinnedSpectra(plan.binning[key], cube, rdxqa, reff=cube.meta['reff'],
  File "/home/ian/miniconda3/envs/dap/lib/python3.9/site-packages/mangadap/proc/spatiallybinnedspectra.py", line 374, in __init__
    self.bin_spectra(cube, rdxqa, reff=reff, ebv=ebv, output_path=output_path,
  File "/home/ian/miniconda3/envs/dap/lib/python3.9/site-packages/mangadap/proc/spatiallybinnedspectra.py", line 1374, in bin_spectra
    bin_indx = self.method['binfunc'](self.rdxqa['SPECTRUM'].data['SKY_COO'][:,0],
  File "/home/ian/miniconda3/envs/dap/lib/python3.9/site-packages/mangadap/proc/spatialbinning.py", line 657, in bin_index
    voronoi_2d_binning(_x, _y, _signal, _noise, self.par['target_snr'],
  File "/home/ian/miniconda3/envs/dap/lib/python3.9/site-packages/vorbin/voronoi_2d_binning.py", line 577, in voronoi_2d_binning
    classe, pixelsize = _accretion(
  File "/home/ian/miniconda3/envs/dap/lib/python3.9/site-packages/vorbin/voronoi_2d_binning.py", line 129, in _accretion
    SN = sn_func(currentBin, signal, noise)
  File "/home/ian/miniconda3/envs/dap/lib/python3.9/site-packages/mangadap/proc/spatialbinning.py", line 534, in sn_calculation_calibrate_noise
    * (1.0 + self.covar*numpy.log10(len(signal[index]))))
TypeError: object of type 'numpy.float64' has no len()

My best guess is that this is occurring because index is just a single pixel in this case and therefore signal[index] has no len(). Is this the intended behavior? If not and this is a bug, I would assume that for the case of a single-pixel 'bin', len(signal[index]) could be replaced by 1.0?

This is all just my best-guess interpretation, I would love to get your insight.

Thanks! Ian

kbwestfall commented 5 months ago

Hi Ian,

Thanks for using the code, and sorry for the very slow reply. We had stopped using the calibration approach in MaNGA a long time ago, and the test coverage isn't good enough to catch a couple of bugs. Thanks for reporting this!

I've implemented a fix in #116 . Would you mind jumping on that branch (covar_approx) and giving it a test run on your data? If it works, I'll merge the PR and release a new tagged version.

Thanks! Kyle

iandroberts commented 4 months ago

Thanks for the patch, I just tested it on one of my cubes and everything seems to be working properly. I think you should be good to merge.

kbwestfall commented 4 months ago

Thanks, Ian! I'll get this merged and upload a new release to pip soon. Closing.