specviz crashes when clicking on filename after selecting ROI

[gderosa2004]

I am loading the COS file lcao2c020_x1dsum.fits (available in /astro/3/jwst_da_sprint_testdata/SpecViz/testdata_201810), and selecting a region well within the spectrum.

Specviz crashes if I try to click on the filename on the left

[gderosa2004]

Same issue if I use the generic fits spectrum, select an ROI and then click on the spectrum

2018-10-24 10:59:29.196 python[92624:2660144] Ignoring request from Finder Sync Extension to register for file:///Users/gderosa/Library/Application%20Support/Box/Box%20Edit/ WARNING: FITSFixedWarning: The WCS transformation has more axes (1) than the image it is associated with (0) [astropy.wcs.wcs] WARNING:astropy:FITSFixedWarning: The WCS transformation has more axes (1) than the image it is associated with (0) Traceback (most recent call last): File "/Users/gderosa/Desktop/specviz/specviz/plugins/statistics/statistics_widget.py", line 255, in update_statistics self.stats = compute_stats(spec) File "/Users/gderosa/Desktop/specviz/specviz/plugins/statistics/statistics_widget.py", line 60, in compute_stats flux = spectrum.flux AttributeError: 'NoneType' object has no attribute 'flux' Abort trap: 6

[nluetzge]

I looked a little bit into this and I think the problem lies in the statics widget. For some reason it does not pull the spectrum. So it is None and then it tries to access the flux from that None and this is causing the crash. So at line 221 when it pulls the spectrum:

spec = self.hub.data_item.spectrum if self.hub.data_item is not None else None

It results in None. There is alot of other weird things with this widget at the moment btw. The display is still showing mostly units instead of numbers for example.

[brechmos-stsci]

I am looking into this issue a bit. @robelgeda is going to fix the display issue https://github.com/spacetelescope/specviz/issues/474

[SaOgaz]

I get this same error when I load a spectrum THEN try to setup a ROI, i'm in dev1557

[nluetzge]

@SaOgaz What do you mean? I also have the spectrum loaded before setting up the ROI. It happens when I have the ROI and then click on the spectrum. It also happens the other way around, I click on the spectrum and then create an ROI.

[brechmos-stsci]

So, the issue with this one is the spec = extract_region(spec, spectral_region) in update_statistics returns None. Sigh, I wrote the extract_region. I'll need to figure out why it is returning None.

[nluetzge]

It doesn't happen with all spectra... might have something to do with the units?

[brechmos-stsci]

So, it has to do with the WCS of the dataset (code from extract_region in specutils):

    if subregion[0].unit.is_equivalent(u.pix):
        left_index = floor(subregion[0].value)
    else:
        left_reg_in_spec_unit = subregion[0].to(spectrum.spectral_axis.unit, u.spectral())
        print('left_reg_in_spec_unit {}'.format(left_reg_in_spec_unit))
        try:
            print(spectrum.wcs)
            left_index = int(np.ceil(spectrum.wcs.world_to_pixel([left_reg_in_spec_unit])))
        except Exception as e:
            left_index = None

The spectrum.wcs here is a <Identity Transform WCS: pixel - Angstrom transformation> but when data is passed in to it to convert it raises an exception. The left_reg_in_spec_unit has a value of 3718.279885088279 Angstrom.

So... looking into that... Not sure why an identity transform would raise an error.

[nluetzge]

I looked at the wcs and it might looks like it is not taking the unit.

> spec.wcs.wcs                                                                                                                                                                 

WCS Keywords

Number of WCS axes: 1
CTYPE : 'LINEAR'  
CRVAL : 200.0  
CRPIX : 0.0  
PC1_1  : 1.0  
CDELT : 1.0  
NAXIS : 0  0

The loader tries to give it a unit at line 75 with:

header['CUNIT1'] = 'Angstrom'

But that does not seem to work... Does the astropy wcs support units? Btw I could reproduce the problem by just using specutils in a script.

[brechmos-stsci]

@nluetzge So, this seems to work with the combined_13330_G130M_v40_bin4.fits dataset. So it is just something about the example_generic.fits dataset that isn't working.

The combined... data file seems to be a more proper fits file (?), the header is very long and full. Also, I do not see a CUNIT field in the header.

The example... data file is very short.

[brechmos-stsci]

As a silly test, I tried copying combined...fits file and removing the CUNIT1='Angstrom' field but that didn't work either. I think this ticket is going to come down to loader issues which you guys already figured out, I am just slow to the party :)

[nluetzge]

@brechmos-stsci I think the difference between those two loaders is how the wcs information is passed. For the COS spectrum the wcs is not passed and only the header is passed as meta:

Spectrum1D(flux=data, spectral_axis=dispersion, uncertainty=uncertainty, meta=meta)

So this is what I get when I look at the wcs:

In [20]: spec.wcs.wcs                                                                                                                                                                
Out[20]: 
<WCS(output_frame=SpectralFrame, input_frame=CoordinateFrame, forward_transform=Model: Tabular1D
Inputs: ('x0',)
Outputs: ('y',)
Parameters: 
  points: (array([   0,    1,    2, ..., 9164, 9165, 9166]),)
  lookup_table: [1124.63035 1124.66915 1124.70795 ... 1480.20388 1480.24268 1480.28148]
  method: linear
  fill_value: nan
  bounds_error: True)>

For the generic fits file the wcs is given like this:

spec = Spectrum1D(flux=data, uncertainty=uncertainty, wcs=wcs, meta=meta)

And the wcs is actually just a wcs:

In [9]: spec.wcs.wcs                                                                                                                                                                 
Out[9]: 
WCS Keywords

Number of WCS axes: 1
CTYPE : 'LINEAR'  
CRVAL : 200.0  
CRPIX : 0.0  
PC1_1  : 1.0  
CDELT : 1.0  
NAXIS : 0  0

So a quick test would be to remove the wcs in the generic fits loader and just give it the header as meta like in the other loader and see if that works.

[nluetzge]

Sorry I was wrong. Its not metaits the spectral_axis keyword that is giving the wcs in the COS case.

[nluetzge]

Ok so its definitely a specutils thing the difference between those to spectra is the gwcs_adapter for the COS spectrum and the fitswcs_adapter for the generic fits file. While the gwcs adapter can handle units in its world_to_pixel routine the fitswcs one cannot (its the astropy wcs which fails with this) however the extract_region routine works with units and gives the values in units to those routines so the fitswcs one fails doing this and then returns none. Maybe @eteq can help here it looks like its a bug in specutils.

[brechmos-stsci]

I can look into it. I have worked on both adapters just before specutils was released.

[nluetzge]

@brechmos-stsci I just realized that we are having two different problems here. The first one was the generic fits file and the second one is the COS file which have the same error but different causes. I played around with my little script I wrote for it and I realized that there is a simple fix to the COS loader for this problem. We have to sort the wavelength first otherwise wcs is complaining. So here is what I added (after line 48 in hst_cos.py):

s = dispersion.argsort()
dispersion = dispersion[s]
data = data[s]
uncertainty = uncertainty[s]

with this it seems to work for me.

[brechmos-stsci]

Huh, though would that work for frequency based spectral axis things (as opposed to wavelength) as they go in the opposite direction? (or is that just visualization?).

[nluetzge]

I think this is just vizualisatio because before the sorting it already displayed the spectrum correctly. I'm building my assumption on this error I'm getting from the wcs:

*** ValueError: The points in dimension 0 must be strictly ascending

[nmearl]

@nluetzge just to clarify, you're trying to load a cos spectrum and you've attempted to do with the both the generic and the cos-specific loaders?

[nluetzge]

This reported issue here happens on two occasions. When loading the actual COS spectrum and when loading the generic fits spectrum (happens not during the loading but once we want to extract a region) both fail because the region returns None. But they have different reasons why that is. And btw we should only work with the example_cos spectrum and not the combined one as this is not the exact format.

[nmearl]

Okay, so in both cases it seems to be an issue with the extract_region machinery in specutils. Specifically, while the extract_region works, it's the _to_edge_pixel calculation that is returning Nones for the bounds, which is then returning an empty spectrum from the extract_region.

[nmearl]

This seems to be consistent with what you were seeing, that the order of the spectral axis is not ascending, so the astropy world2pix method fails. (Sorry, I realize you've all combed through this, I'm just talking myself through the issue. 😅 )

[nluetzge]

Yes see comment of @brechmos-stsci :

So, the issue with this one is the spec = extract_region(spec, spectral_region) in update_statistics returns None. Sigh, I wrote the extract_region. I'll need to figure out why it is returning None.

[nmearl]

Okay, this is quite COS-specific in that that the two detectors have a slight overlapping region, which is why when the array is flattened in the loader, it's still not considered in any sorted order. Sorting the dispersion and data arrays prior fixes the issue.

Likewise, I'm not having trouble loading the generic_spectrum.fits, plopping down a region, and getting statistics about the region. @nluetzge are you seeing the exact same failure in the statistics widget in this case?

[eteq]

Okay, this is quite COS-specific in that that the two detectors have a slight overlapping region

This raises a question about who's "job" it is to check the order of the spectrum - the loader (i.e., the user in interactive python use) or the Spectrum1D initializer. That's primarily a specutils question, though, so I will make in issue in specutils about that and link it here. Either way the specviz fix can just be to have the loader do it.

[nmearl]

@eteq agreed. #499 contains the quick sort fix.

[nluetzge]

@nmearl Yes I do see the same issue with the generic. And its the exact same error. I open specviz, load the generic spectrum, create a region, click on the spectrum tab on the left, it crashes with:

Traceback (most recent call last):
  File "/Users/nluetzge/JWST/dev_tools/specviz/specviz/specviz/plugins/statistics/statistics_widget.py", line 255, in update_statistics
    self.stats = compute_stats(spec)
  File "/Users/nluetzge/JWST/dev_tools/specviz/specviz/specviz/plugins/statistics/statistics_widget.py", line 60, in compute_stats
    flux = spectrum.flux
AttributeError: 'NoneType' object has no attribute 'flux'
Abort trap: 6

Maybe we have a different version of Astropy? Also here is a script to reproduce the error. In my case the final region returns None:

from astropy.io import fits
from astropy.units import Unit
from astropy.nddata import StdDevUncertainty
from astropy.wcs import WCS
from astropy.table import Table
from specutils.spectra.spectral_region import SpectralRegion
from specutils.manipulation import extract_region
from specutils import Spectrum1D

file = 'data/example_generic.fits'
hdulist = fits.open(file)
header = hdulist[0].header
header['CTYPE1'] = 'LINEAR'
header['CUNIT1'] = 'Angstrom'
header['NAXIS'] = 1
tab = Table.read(file)
header['NAXIS1'] = len(tab)
unit = Unit('Jy')
meta = {'header': header}
wcs = WCS(header)
uncertainty = StdDevUncertainty(tab["err"] * unit)
data = tab["flux"] * unit
hdulist.close()
spec = Spectrum1D(flux=data, wcs=wcs, uncertainty=uncertainty, meta=meta)

sr = SpectralRegion(500*Unit('Angstrom'), 10000*Unit('Angstrom'))

print(sr)

sub_spectrum = extract_region(spec, sr)

print(sub_spectrum)

[nmearl]

Hmm, running your script succeeds for me:

[nluetzge]

Ok crazy. Whats your astropy version? I just updated mine but it still gives the same error.

[nluetzge]

Or maybe you are using a different specutils version?

[gderosa2004]

@nmearl I see the same thing as Nora:

Spectral Region, 1 sub-regions:
  (500.0 Angstrom, 10000.0 Angstrom) 

None

[nmearl]

(specviz_dev) ➜  Downloads python -c "import specutils; print(specutils.__version__)"
0.4
(specviz_dev) ➜  Downloads python -c "import astropy; print(astropy.__version__)"
3.0.2

Updating astropy to 3.0.5 does not trigger the failure.

[nluetzge]

This is so weird. I also have 0.4 and 3.0.5 ... do we have different spectra? Where did you take the example_generic.fits from? Also it looks like it is not 100% extracting the right region in your case. It is supposed to go from 500 - 10000 Angstrom but your region goes from 500 - 2.02e4

[javerbukh]

This issue no longer happens after #499 was merged. Has this become a discussion thread or can this be closed?

[brechmos-stsci]

If fixed AND because things are going to change for the "new way of doing loaders" I think this can be closed. With the new way Sara is working on, all files we have will have to be tested anyway.

That being said, it seems like there should be more checks in the stats part to see if there are errors coming back from the region extraction and/or stats calculations.

[nmearl]

This should not be closed yet as @nluetzge and @gderosa2004 are still having issues with some of the data sets and we're actively trying to resolve those.

I'm not sure what this "new way doing of loaders" is, but the loader wizard implementation that Sarah, myself, Jesse, and Erik will be meeting later today about is just a graphical interface to creating the same loader files that are used to load these files, so this is still relevant.

[nluetzge]

Yup still having the issue with the generic fits file even after the merge. But the problem with the COS spectrum is solved now.

[nluetzge]

@nmearl Can you try with the spectrum @gderosa2004 put here #462 ? Its the only thing I could think of that is different. @eteq also get the None when running my script. If its the case and it actually is the spectrum it will be easier to trace the problem.

[gderosa2004]

@nluetzge, @nmearl which spectrum are we talking about?

[nluetzge]

The example_generic.fits

[nluetzge]

So I think the remaining issue here is currently being worked on by @brechmos-stsci in astropy/specutils#367 but I think we should keep it open until the issue is resolved in specutils.