Unit conversion: Specviz silently ignoring spectrum with almost compatible units

[Jdaviz-Triage-Bot]

Reporter: Camilla Pacifici

I am playing with the redshift notebook to include unit conversion in specviz.

The observed spectrum is in MJy/sr which is almost compatible with f_lam units (erg/s/cm2/A). If I pass the observed spectrum to Specviz first and then a template spectrum in f_lam, Specviz silently accepts the spectrum (it is in the data menu), but it does not visualize it.

If I pass the observed spectrum in Jy, everything works beautifully!

🐱

---

DISCLAIMER: This issue was autocreated by the Jdaviz Issue Creation Bot on behalf of the reporter.

[pllim]

The /sr is the problem.

What did you expect should happen? An outright traceback?

[camipacifici]

I would have expected a warning. The logger was all green.

[pllim]

Actually the fix would have to be deep in glue-core. The unit conversion happens here:

https://github.com/glue-viz/glue/blob/1a5c7676c025a1a025068b806f6f90ed53bba543/glue/viewers/profile/state.py#L478-L479

And I think this silently skips any incompatible unit:

https://github.com/glue-viz/glue/blob/1a5c7676c025a1a025068b806f6f90ed53bba543/glue/viewers/profile/state.py#L134

xref glue-viz/glue#2296

[pllim]

That said, we could probably have a workaround in our own parser to check unit in advance and throw exception, but I feel like that defeats the purpose of glue-core implementation.

[astrofrog]

This might be an issue beyond just unit conversion that in general incompatible layers are not really indicated in any way? (basically whether the layer artist enabled property is True/False). In the Qt application when a layer is incompatible it is possible to see this in the UI with a message, so we should think about how to do the same in glue-jupyter and/or jdaviz. In any case I agree this is something that needs fixing in glue-jupyter.

[dhomeier]

If I pass the observed spectrum in Jy, everything works beautifully!

You mean it is automatically matched to the template spectrum in erg/s/cm2/A? Is Jdaviz then already setting custom default converters? It is of course straightforward, but I see nothing like u.spectral set in SimpleAstropyUnitConverter...

However as @pllim pointed out, Jy/sr is not compatible without integration or multiplication over solid angle, so I don't see any way to setup a default conversion for this. But I agree that the UnitConversionError should be caught separately to issue a warning or notification to set up a custom converter.

[dhomeier]

In the Qt application when a layer is incompatible it is possible to see this in the UI with a message, so we should think about how to do the same in glue-jupyter and/or jdaviz. In any case I agree this is something that needs fixing in glue-jupyter.

So should
https://github.com/glue-viz/glue/blob/1a5c7676c025a1a025068b806f6f90ed53bba543/glue/viewers/profile/state.py#L131-L134
also provide some diagnostic information on e.g. IncompatibleDataException (anything else?), and will warnings.warn() do, or did you have something else in mind for it?

[dhomeier]

All sensible (or rather physically possible) unit conversions seem already enabled, e.g. if I manually set the template as I_lambda

spec_unit = u.Unit('erg / (cm^2*s*Angstrom*steradian)')
# Create Spectrum1D object
template = Spectrum1D(spectral_axis=(template['col1'] * u.Angstrom).to(u.um), 
                      flux=template['col2'] * 1E8 * template['col1']**-2 * spec_unit)

it is correctly plotted with viz3 (that is the step you were testing?)
So setting a compatible unit here is the responsibility of the user imo; this leaves the issue of emitting a useful warning from glue-core if it cannot use the dataset.

[pllim]

emitting a useful warning from glue-core

@dhomeier , do you have any idea how to do that in a way that is acceptable by glue-core? I don't want to waste time working on a patch only to find out it is too specific to Jdaviz or something.

[dhomeier]

I think warnings about incompatible units would be generally useful enough, just not sure how to communicate them to the Specviz app. In the standard notebook, a simple warnings.warn is not forwarded – or it is skipped someplace else than
https://github.com/glue-viz/glue/blob/1a5c7676c025a1a025068b806f6f90ed53bba543/glue/viewers/profile/state.py#L131-L134

also provide some diagnostic information on e.g. IncompatibleDataException (anything else?), and will warnings.warn() do, or did you have something else in mind for it?

I suppose that could also be generally useful enough, though I don't know how such exceptions would typically look like.

[pllim]

@dhomeier , you are right. I cannot get warning or exception to raise from that line. Now I question reality. Digging more.

[pllim]

Okay, sorry that I led you down the wrong rabbit hole. It turns out we never link the flux anyway, so the converter is not getting used (as far as I can understand). This might be something we have to patch here in Jdaviz after all.

Here is a minimal test case:

from astropy import units as u
from specutils import Spectrum1D

from jdaviz import Specviz

wav = [1.1, 1.2, 1.3] * u.um
sp1 = Spectrum1D(flux=[1, 1.1, 1] * (u.MJy / u.sr), spectral_axis=wav)
sp2 = Spectrum1D(flux=[1, 1, 1.1] * (u.MJy), spectral_axis=wav)
flux3 = ([1, 1.1, 1] * u.MJy).to(u.erg / u.s / u.cm / u.cm / u.AA, u.spectral_density(wav))
sp3 = Spectrum1D(flux=flux3, spectral_axis=wav)

specviz = Specviz()
specviz.show()

specviz.load_data(sp2, data_label="2")  # OK
specviz.load_data(sp1, data_label="1")  # Not OK
specviz.load_data(sp3, data_label="3")  # OK

[pllim]

@astrofrog or @dhomeier , is #2485 sufficient or is there a better way?