hzovaro / spaxelsleuth

A package for analysing data from large integral field unit surveys such as the SAMI and Hector Galaxy Surveys.
MIT License
1 stars 1 forks source link

Add wavelength range checks to utility functions #38

Closed hzovaro closed 8 months ago

hzovaro commented 8 months ago

As per title. Return arrays of NaNs if the desired quantities cannot be computed due to the wavelength range covered by the input wavelength arrays.

hzovaro commented 8 months ago
hzovaro commented 8 months ago

The above NaN issue was being caused by velocity.get_slices_in_velocity_range. I've fixed it by making a copy of v_map within the function.

hzovaro commented 8 months ago

New to do: figure out what fixing this bug has affected...

hzovaro commented 8 months ago

I ran sami_regression_test.py on the recom/default test DataFrames, which showed the following columns to be affected:

These changed because:

  1. In sami.py, v_map[0] (the narrow gas velocity component) was being passed to compute_continuum_intensity() (line 638).
  2. In compute_continuum_intensity(), we called get_slices_in_velocity_range() on v_map[0], which was overwriting NaNs with zeros.
  3. We then called compute_v_grad() on v_map, which was incorrectly computing v_grad (component 1) which relies on v_map[0] (and therefore affecting Beam smearing flag (component 1)), creating non-NaN entries where there should have been NaNs.
  4. Next we called compute_measured_HALPHA_amplitude_to_noise() which uses v_map[0]. In this function as originally written, NaNs in v_map[0] should propagate to NaNs in HALPHA A/N (measured) because they are used to get wavelength values via get_wavelength_from_velocity() (which does NOT replace NaNs with zeros). However, because we had inadvertently replaced NaNs with zeros in v_map, we were basically adopting LOS velocities of 0 when calculating the wavelength range over which to evaluate the A/N, which is actually desired behaviour!!

This means that this bug only affected v_grad measurements (which have not been used in any science so far) and that the A/N measurements have "accidentally" been correct this whole time.

Fixes/TODOs:

hzovaro commented 8 months ago

Merged into hector, will merge hector into dev when complete.