OttoStruve / muler

A Python package for working with pipeline-produced spectra from IGRINS, HPF, and Keck NIRSPEC
https://muler.readthedocs.io
MIT License
15 stars 9 forks source link

JOSS Review - Sky subtraction notebook - UnitConversionError #94

Closed wtgee closed 2 years ago

wtgee commented 2 years ago

I'm getting some unit conversion errors in the Refined HPF sky subtraction notebook. I've installed according to the installation guide so have a conda environment built from the environment.yaml file.

In [5]: ax = spectrum.plot(ylo=0.6, yhi=1.3, label='Observed')
        spectrum.sky_subtract(method='scalar').plot(ax=ax, label='Sky Subtracted')
        ax.legend();

---------------------------------------------------------------------------
UnitConversionError                       Traceback (most recent call last)
File ~/miniforge3/envs/muler_dev/lib/python3.8/site-packages/astropy/units/quantity_helper/helpers.py:68, in get_converters_and_unit(f, unit1, unit2)
     67 try:
---> 68     converters[changeable] = get_converter(unit2, unit1)
     69 except UnitsError:

File ~/miniforge3/envs/muler_dev/lib/python3.8/site-packages/astropy/units/quantity_helper/helpers.py:32, in get_converter(from_unit, to_unit)
     30 """Like Unit._get_converter, except returns None if no scaling is needed,
     31 i.e., if the inferred scale is unity."""
---> 32 converter = from_unit._get_converter(to_unit)
     33 return None if converter is unit_scale_converter else converter

File ~/miniforge3/envs/muler_dev/lib/python3.8/site-packages/astropy/units/core.py:1064, in UnitBase._get_converter(self, other, equivalencies)
   1062                 pass
-> 1064 raise exc

File ~/miniforge3/envs/muler_dev/lib/python3.8/site-packages/astropy/units/core.py:1049, in UnitBase._get_converter(self, other, equivalencies)
   1048 try:
-> 1049     return self._apply_equivalencies(
   1050         self, other, self._normalize_equivalencies(equivalencies))
   1051 except UnitsError as exc:
   1052     # Last hope: maybe other knows how to do it?
   1053     # We assume the equivalencies have the unit itself as first item.
   1054     # TODO: maybe better for other to have a `_back_converter` method?

File ~/miniforge3/envs/muler_dev/lib/python3.8/site-packages/astropy/units/core.py:1025, in UnitBase._apply_equivalencies(self, unit, other, equivalencies)
   1023 other_str = get_err_str(other)
-> 1025 raise UnitConversionError(
   1026     f"{unit_str} and {other_str} are not convertible")

UnitConversionError: 'ct' and '' (dimensionless) are not convertible

During handling of the above exception, another exception occurred:

UnitConversionError                       Traceback (most recent call last)
Input In [5], in <cell line: 2>()
      1 ax = spectrum.plot(ylo=0.6, yhi=1.3, label='Observed')
----> 2 spectrum.sky_subtract(method='scalar').plot(ax=ax, label='Sky Subtracted')
      3 ax.legend()

File /storage/wtgee/Projects/JOSS/muler/src/muler/hpf.py:336, in HPFSpectrum.sky_subtract(self, method)
    334 # These steps should propagate uncertainty?
    335 sky_estimator = spec.sky.multiply(beta, handle_meta="first_found")
--> 336 return spec.subtract(sky_estimator, handle_meta="first_found")

File ~/miniforge3/envs/muler_dev/lib/python3.8/site-packages/astropy/nddata/mixins/ndarithmetic.py:524, in NDArithmeticMixin.subtract(self, operand, operand2, **kwargs)
    521 @sharedmethod
    522 @format_doc(_arit_doc, name='subtraction', op='-')
    523 def subtract(self, operand, operand2=None, **kwargs):
--> 524     return self._prepare_then_do_arithmetic(np.subtract, operand, operand2,
    525                                             **kwargs)

File ~/miniforge3/envs/muler_dev/lib/python3.8/site-packages/astropy/nddata/mixins/ndarithmetic.py:613, in NDArithmeticMixin._prepare_then_do_arithmetic(self_or_cls, operation, operand, operand2, **kwargs)
    610 operand2 = cls(operand2)
    612 # Now call the _arithmetics method to do the arithmetics.
--> 613 result, init_kwds = operand._arithmetic(operation, operand2, **kwargs)
    615 # Return a new class based on the result
    616 return cls(result, **init_kwds)

File ~/miniforge3/envs/muler_dev/lib/python3.8/site-packages/astropy/nddata/mixins/ndarithmetic.py:245, in NDArithmeticMixin._arithmetic(self, operation, operand, propagate_uncertainties, handle_mask, handle_meta, uncertainty_correlation, compare_wcs, **kwds)
    240     kwargs['wcs'] = self._arithmetic_wcs(operation, operand,
    241                                          compare_wcs, **kwds2['wcs'])
    243 # Then calculate the resulting data (which can but not needs to be a
    244 # quantity)
--> 245 result = self._arithmetic_data(operation, operand, **kwds2['data'])
    247 # Determine the other properties
    248 if propagate_uncertainties is None:

File ~/miniforge3/envs/muler_dev/lib/python3.8/site-packages/astropy/nddata/mixins/ndarithmetic.py:320, in NDArithmeticMixin._arithmetic_data(self, operation, operand, **kwds)
    317     result = operation(self.data << self.unit,
    318                        operand.data << dimensionless_unscaled)
    319 else:
--> 320     result = operation(self.data << self.unit,
    321                        operand.data << operand.unit)
    323 return result

File ~/miniforge3/envs/muler_dev/lib/python3.8/site-packages/astropy/units/quantity.py:466, in Quantity.__array_ufunc__(self, function, method, *inputs, **kwargs)
    444 """Wrap numpy ufuncs, taking care of units.
    445 
    446 Parameters
   (...)
    460     Results of the ufunc, with the unit set properly.
    461 """
    462 # Determine required conversion functions -- to bring the unit of the
    463 # input to that expected (e.g., radian for np.sin), or to get
    464 # consistent units between two inputs (e.g., in np.add) --
    465 # and the unit of the result (or tuple of units for nout > 1).
--> 466 converters, unit = converters_and_unit(function, method, *inputs)
    468 out = kwargs.get('out', None)
    469 # Avoid loop back by turning any Quantity output into array views.

File ~/miniforge3/envs/muler_dev/lib/python3.8/site-packages/astropy/units/quantity_helper/converters.py:174, in converters_and_unit(function, method, *args)
    171 units = [getattr(arg, 'unit', None) for arg in args]
    173 # Determine possible conversion functions, and the result unit.
--> 174 converters, result_unit = ufunc_helper(function, *units)
    176 if any(converter is False for converter in converters):
    177     # for multi-argument ufuncs with a quantity and a non-quantity,
    178     # the quantity normally needs to be dimensionless, *except*
   (...)
    181     # can just have the unit of the quantity
    182     # (this allows, e.g., `q > 0.` independent of unit)
    183     try:
    184         # Don't fold this loop in the test above: this rare case
    185         # should not make the common case slower.

File ~/miniforge3/envs/muler_dev/lib/python3.8/site-packages/astropy/units/quantity_helper/helpers.py:70, in get_converters_and_unit(f, unit1, unit2)
     68     converters[changeable] = get_converter(unit2, unit1)
     69 except UnitsError:
---> 70     raise UnitConversionError(
     71         "Can only apply '{}' function to quantities "
     72         "with compatible dimensions"
     73         .format(f.__name__))
     75 return converters, unit1

UnitConversionError: Can only apply 'subtract' function to quantities with compatible dimensions
wtgee commented 2 years ago

It's never too encouraging when you see a developer comment of "# Last hope: maybe other knows how to do it?" in an exception from a library (astropy) you are using. πŸ˜ƒ

gully commented 2 years ago

Thank you @wtgee πŸ™ I will dig into this!! I've seen this crop up before and it may be a dependency conflict, but I will spot check again.

gully commented 2 years ago

closed by #97 ! πŸ₯³