search

spacetelescope / jdaviz

JWST astronomical data analysis tools in the Jupyter platform
https://jdaviz.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
134 stars 71 forks source link

BUG: Uncertainty type ignored in Cubeviz spectral extraction #2713

Open pllim opened 6 months ago

pllim commented 6 months ago

The current documentation (at the time of original posting this issue) at https://jdaviz.readthedocs.io/en/latest/cubeviz/plugins.html#spectral-extraction does not mention at all how uncertainty and mask arrays are used in the extraction process. The mask is yet another issue (#2714); here I am only focusing on uncertainty:

https://github.com/spacetelescope/jdaviz/blob/b563ed3e4090d7f8d752c54a01b8fcb222811ef2/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py#L207-L222

This was originally implemented by @bmorris3 via #2039 . In the code above, uncertainty cube is assumed to be always be in standard deviation.

However, Cubeviz parser actually allows at least 3 different kinds of uncertainty (inverse variance, variance, standard deviation, and who knows what else). The uncertainty type was only to detect whether a cube falls in that category, we do not have any extra processing to make sure it is actually always standard deviation.

https://github.com/spacetelescope/jdaviz/blob/b563ed3e4090d7f8d752c54a01b8fcb222811ef2/jdaviz/configs/cubeviz/plugins/parsers.py#L19-L21

As a result, if the uncertainty cube is not really standard deviation, it might be wrong to assume so, and that wrong assumption would propagate to it being handled improperly during extraction.

While, there appears to be some checks in https://github.com/glue-viz/glue-astronomy/blob/main/glue_astronomy/translators/nddata.py . I am not sure if those checks are actually used always due to the way we store uncertainty cube as a separate Data object, and not as data.uncertainty.

For instance, at one point, glue-astronomy translator looks for data.meta.get('uncertainty_type', 'std') but we never set that in Cubeviz parser. As a result, any of our code that goes through that logic would always return 'std' regardless.

The scope of this ticket is to confirm whether or not my concern of a bug is real. And if it is, what is necessary to fix. Thanks.

🐱

bmorris3 commented 6 months ago

The glue-astronomy NDData translator will represent any uncertainties in glue Data entry for the uncertainty as StdDevUncertainty by default (see NDData source here) when you ask for data.to_object. But note that the relevant translator from input->Data depends on what the input data are. I believe Cubeviz spectral extraction returns NDDataArrays, but the jwst products from MAST will be parsed via the glue-astro translator for Spectrum1D objects (in this block).

Both the NDData and Spectrum1D translators check for a metadata keyword called uncertainty_type, and uses the associated value to determine which type of uncertainties are given (source here). I believe I implemented this approach knowing that we could set an uncertainty_type key in meta in the parser, if we knew how to identify the uncertainty type correctly.

For your ticket, the next step is to check if (1) any JWST cubes have non-stddev uncertainties, (2) if there's a keyword that specifies the uncertainty type in the JWST spectral cubes used in Cubeviz, (3) setting data.meta['uncertainty_type'] if you find (2) and it's not stddev.