Add CASA tests - Githubissues

keflavich commented 5 years ago

These have been sitting on my HD for ages... we agree w/CASA's feather to within numerical precision (1e-7 ish) for a reasonable set of parameters.

e-koch commented 2 years ago

Tests are running, but the comparison with CASA feather is failing badly. Need to sort out where something went wrong.

e-koch commented 2 years ago

@keflavich -- The weirdness is coming from the make_extended I swapped in from turbustat (since it returns a pure powerlaw centered at 0.).

I swapped it back for now to eventually get the tests passing. There's likely a unit mismatch now but I think visually things look like they should.

UPDATE: We're back to using the make_extended from turbustat that guarantees a power-law with no imaginary component. The issue was a unit conversion for Jy/beam, as noted below.

e-koch commented 2 years ago

Many updates and some bugs we caught in the comparison test to CASA:

BUG: Need to convert low res data in Jy/beam into the Jy/beam for the high res beam. Introduced only recently in #28. It was handled correctly before.
We now generate the power-law data with Jy/sr, then appropriately convert to Jy/beam for the mock SD and interferometer data.
Ensure the right header information is included in the test data to have everything CASA wants. Most notably, a spec axis needs to be defined for 2D images as feather grabs the frequency from there and uses that to estimate the eff. single dish diameter, based on the SD beam. Without that data, CASA appears to create a default spec axis centered near 1.4 GHz (so probably the HI line). We were mocking the data to be at 100 GHz. This doesn't impact the actual test case when the minimum spectral information is provided to CASA (which we do in importfits).

e-koch commented 2 years ago

Phew. Tests are finally passing. @keflavich can you have a second look through? We should prioritize merging due to the Jy/beam conversion bug.

Just to note, I've changed the CASA to uvcombine feather comparison metric to be the median fractional difference within 5e-4. Both the feathered maps have outliers at the lowest values in the original image, and these are in slightly different locations.

keflavich commented 2 years ago

lgtm, I'm just trying to run things locally before I hit merge... and finding out that yet another computer needs to have All The Things updated.

keflavich commented 2 years ago

Does yours look like this? I think there's a single zero in the 'original' data set that results in a nan in the (a-b)/c image

e-koch commented 2 years ago

Ah, right. There's a img - img.min() in the plaw image generation giving the min value as 0. I've changed that to 1.1 * img.min() to ensure the min is still positive.

keflavich commented 2 years ago

I'm back on the other machine today and now the script doesn't even run? I'm a bit lost, but I'm doing this between telecons so never putting full brain on it:

Traceback (most recent call last):
  File "/home/adam/anaconda3/envs/python3.8/lib/python3.8/site-packages/astropy/units/quantity_helper/helpers.py", line 68, in get_converters_and_unit
    converters[changeable] = get_converter(unit2, unit1)
  File "/home/adam/anaconda3/envs/python3.8/lib/python3.8/site-packages/astropy/units/quantity_helper/helpers.py", line 32, in get_converter
    converter = from_unit._get_converter(to_unit)
  File "/home/adam/anaconda3/envs/python3.8/lib/python3.8/site-packages/astropy/units/core.py", line 1066, in _get_converter
    raise exc
  File "/home/adam/anaconda3/envs/python3.8/lib/python3.8/site-packages/astropy/units/core.py", line 1051, in _get_converter
    return self._apply_equivalencies(
  File "/home/adam/anaconda3/envs/python3.8/lib/python3.8/site-packages/astropy/units/core.py", line 1027, in _apply_equivalencies
    raise UnitConversionError(
UnitConversionError: 'Jy / beam' and 'MJy / sr' are not convertible

During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/adam/repos/uvcombine/examples/example_uvcombine_casa_feather_comparison.py", line 132, in <module>
    diff = (uvcomb_feather_proj - casa_feather_proj).value
  File "/home/adam/anaconda3/envs/python3.8/lib/python3.8/site-packages/astropy/units/quantity.py", line 594, in __array_ufunc__
    converters, unit = converters_and_unit(function, method, *inputs)
  File "/home/adam/anaconda3/envs/python3.8/lib/python3.8/site-packages/astropy/units/quantity_helper/converters.py", line 174, in converters_and_unit
    converters, result_unit = ufunc_helper(function, *units)
  File "/home/adam/anaconda3/envs/python3.8/lib/python3.8/site-packages/astropy/units/quantity_helper/helpers.py", line 70, in get_converters_and_unit
    raise UnitConversionError(
UnitConversionError: Can only apply 'subtract' function to quantities with compatible dimensions

this is not caused by your latest change

keflavich commented 2 years ago

this was my mistake again; I had made a local change that caused the break.

Now looks good:

radio-astro-tools / uvcombine

Add CASA tests #23