mne-tools / mne-connectivity

Connectivity algorithms that leverage the MNE-Python API.
https://mne.tools/mne-connectivity/dev/index.html
BSD 3-Clause "New" or "Revised" License
68 stars 34 forks source link

iPython can't display the spectral connectivity classes #39

Closed drammock closed 3 years ago

drammock commented 3 years ago

Describe the bug

printing an instance of the spectral connectivity classes in an iPython terminal results in an iPython error

Steps to reproduce

con = spectral_connectivity(...)
con

Actual results

In [6]: con
Out[6]: ---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
/opt/miniforge3/envs/mnedev/lib/python3.9/site-packages/IPython/core/formatters.py in __call__(self, obj)
    700                 type_pprinters=self.type_printers,
    701                 deferred_pprinters=self.deferred_printers)
--> 702             printer.pretty(obj)
    703             printer.flush()
    704             return stream.getvalue()

/opt/miniforge3/envs/mnedev/lib/python3.9/site-packages/IPython/lib/pretty.py in pretty(self, obj)
    392                         if cls is not object \
    393                                 and callable(cls.__dict__.get('__repr__')):
--> 394                             return _repr_pprint(obj, self, cycle)
    395 
    396             return _default_pprint(obj, self, cycle)

/opt/miniforge3/envs/mnedev/lib/python3.9/site-packages/IPython/lib/pretty.py in _repr_pprint(obj, p, cycle)
    698     """A pprint that just redirects to the normal repr function."""
    699     # Find newlines and replace them with p.break_()
--> 700     output = repr(obj)
    701     lines = output.splitlines()
    702     with p.group():

/opt/mne/connectivity/mne_connectivity/base.py in __repr__(self)
    293         r += f', nodes, n_estimated : {self.n_nodes}, ' \
    294              f'{self.n_estimated_nodes}'
--> 295         r += ', ~%s' % (sizeof_fmt(self._size),)
    296         r += '>'
    297         return r

/opt/mne/connectivity/mne_connectivity/base.py in _size(self)
    502         size = 0
    503         size += object_size(self.get_data())
--> 504         size += object_size(self.attrs)
    505         return size
    506 

/opt/mne/python/mne/utils/numerics.py in object_size(x, memo)
    704         for key, value in x.items():
    705             size += object_size(key, memo)
--> 706             size += object_size(value, memo)
    707     elif isinstance(x, (list, tuple)):
    708         size = sys.getsizeof(x) + sum(object_size(xx, memo) for xx in x)

/opt/mne/python/mne/utils/numerics.py in object_size(x, memo)
    713                    for xx in [x, x.data, x.indices, x.indptr])
    714     else:
--> 715         raise RuntimeError('unsupported type: %s (%s)' % (type(x), x))
    716     memo[id_] = size
    717     return size

RuntimeError: unsupported type: <class 'mne_connectivity.spectral._WPLIDebiasedEst'> (<mne_connectivity.spectral._WPLIDebiasedEst object at 0x7f4a5fe8f940>)
> /opt/mne/python/mne/utils/numerics.py(715)object_size()
    713                    for xx in [x, x.data, x.indices, x.indptr])
    714     else:
--> 715         raise RuntimeError('unsupported type: %s (%s)' % (type(x), x))
    716     memo[id_] = size
    717     return size
adam2392 commented 3 years ago

I think this is also closed by #43 because method was being passed a function rather than a string.

Lmk @drammock ?

drammock commented 3 years ago

in trying to test this out, I get the following traceback from read_connectivity

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-2-dac627b44b01> in <module>
----> 1 foo = read_connectivity('/data/ftof/results/envelope-correlations/f2f_009-attend-beta-band-envelope-correlation.nc')

/opt/mne/connectivity/mne_connectivity/io.py in read_connectivity(fname)
     59     """
     60     # open up a data-array using xarray
---> 61     conn_da = xr.open_dataarray(fname, engine='h5netcdf')
     62 
     63     # map 'n/a' to 'None'

/opt/miniforge3/envs/mnedev/lib/python3.9/site-packages/xarray/backends/api.py in open_dataarray(filename_or_obj, engine, chunks, cache, decode_cf, mask_and_scale, decode_times, decode_timedelta, use_cftime, concat_characters, decode_coords, drop_variables, backend_kwargs, *args, **kwargs)
    652         )
    653 
--> 654     dataset = open_dataset(
    655         filename_or_obj,
    656         decode_cf=decode_cf,

/opt/miniforge3/envs/mnedev/lib/python3.9/site-packages/xarray/backends/api.py in open_dataset(filename_or_obj, engine, chunks, cache, decode_cf, mask_and_scale, decode_times, decode_timedelta, use_cftime, concat_characters, decode_coords, drop_variables, backend_kwargs, *args, **kwargs)
    481         engine = plugins.guess_engine(filename_or_obj)
    482 
--> 483     backend = plugins.get_backend(engine)
    484 
    485     decoders = _resolve_decoders_kwargs(

/opt/miniforge3/envs/mnedev/lib/python3.9/site-packages/xarray/backends/plugins.py in get_backend(engine)
    154         engines = list_engines()
    155         if engine not in engines:
--> 156             raise ValueError(
    157                 f"unrecognized engine {engine} must be one of: {list(engines)}"
    158             )

ValueError: unrecognized engine h5netcdf must be one of: ['netcdf4', 'scipy', 'store']
drammock commented 3 years ago

can you test it, or (better) write a test as part of #43 that failed before and passes now?

adam2392 commented 3 years ago

Ah okay, I didn't update the what's_new with a new requirement.

This error should be fixed with the unit test I added in #43 with the new saving that occurs via a different engine.

You need to run pip install h5netcdf

drammock commented 3 years ago

still no love...

In [1]: from mne_connectivity import read_connectivity
In [2]: foo = read_connectivity('/data/ftof/results/envelope-correlations/f2f_009-attend-beta-band-envelope-correlation.nc')
In [3]: foo
Out[3]: ---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/opt/miniforge3/envs/mnedev/lib/python3.9/site-packages/IPython/core/formatters.py in __call__(self, obj)
    700                 type_pprinters=self.type_printers,
    701                 deferred_pprinters=self.deferred_printers)
--> 702             printer.pretty(obj)
    703             printer.flush()
    704             return stream.getvalue()

/opt/miniforge3/envs/mnedev/lib/python3.9/site-packages/IPython/lib/pretty.py in pretty(self, obj)
    392                         if cls is not object \
    393                                 and callable(cls.__dict__.get('__repr__')):
--> 394                             return _repr_pprint(obj, self, cycle)
    395 
    396             return _default_pprint(obj, self, cycle)

/opt/miniforge3/envs/mnedev/lib/python3.9/site-packages/IPython/lib/pretty.py in _repr_pprint(obj, p, cycle)
    698     """A pprint that just redirects to the normal repr function."""
    699     # Find newlines and replace them with p.break_()
--> 700     output = repr(obj)
    701     lines = output.splitlines()
    702     with p.group():

/opt/mne/connectivity/mne_connectivity/base.py in __repr__(self)
    289         r += f', nodes, n_estimated : {self.n_nodes}, ' \
    290              f'{self.n_estimated_nodes}'
--> 291         r += ', ~%s' % (sizeof_fmt(self._size),)
    292         r += '>'
    293         return r

/opt/mne/connectivity/mne_connectivity/base.py in _size(self)
    497         """Estimate the object size."""
    498         size = 0
--> 499         size += object_size(self.get_data())
    500         size += object_size(self.attrs)
    501         return size

/opt/mne/connectivity/mne_connectivity/base.py in get_data(self, output)
    559                     data[row_idx, col_idx, ...] = self._data
    560             elif self.indices == 'symmetric':
--> 561                 data = np.zeros(new_shape)
    562 
    563                 # get the upper/lower triangular indices

TypeError: only integer scalar arrays can be converted to a scalar index
larsoner commented 3 years ago

Looks like two bugs:

  1. object_size(self.get_data()) should probably be object_size(self.get_data('compact') or whatever the "get array as the native storage) is. This could also be self._data.asarray() or whatever if _data is an XArray and .asarray() will give you a pointer/view to the underlying ndarray (if such a thing exists). Basically you want to know the bytes in memory, whereas self.get_data() will actually create a new copy.
  2. Clearly there is some bug with obj.get_data() for this obj. @drammock can you import pdb; pdb.pm() to p new_shape so that @adam2392 can see what could be causing this? I wonder if it's an EpochConnectivity with zero usable epochs for example, I feel like I ran into a bug with envelope_correlation where iterating over data with size 0 was problematic and didn't get around to opening an issue yet...
drammock commented 3 years ago
(Pdb) p new_shape
[array([26]), array([26]), 1]
larsoner commented 3 years ago

Okay three bugs.

  1. get data being used, as above (maybe could just use raveled?).
  2. >>> x = np.zeros((0, 2, 2))
    >>> mne_connectivity.envelope_correlation(x)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "<decorator-gen-546>", line 24, in envelope_correlation
      File "/home/larsoner/python/mne-connectivity/mne_connectivity/envelope.py", line 167, in envelope_correlation
        triu_inds = np.triu_indices(n_nodes, k=0)
    UnboundLocalError: local variable 'n_nodes' referenced before assignment
  3. Whatever gave the class @drammock managed to create. @drammock can you try to make a MWE for creating a Connectivity class whose .get_data() raises this error (and thus __repr__ will too)?
adam2392 commented 3 years ago

I'm still not able to reproduce the error. The following unit test currently works for me.

def test_roundtrip_envelope_correlation(tmp_path):
    """Test write/read roundtrip for envelope correlation."""
    rng = np.random.RandomState(0)
    n_epochs, n_signals, n_times = 1, 4, 64
    data = rng.randn(n_epochs, n_signals, n_times)
    data_hilbert = hilbert(data, axis=-1)
    corr = envelope_correlation(data_hilbert)
    tmp_file = tmp_path / 'temp_file.nc'
    corr.save(tmp_file)

    read_corr = read_connectivity(tmp_file)
    assert repr(corr) == repr(read_corr)

Okay three bugs.

  1. 
    >>> x = np.zeros((0, 2, 2))
    >>> mne_connectivity.envelope_correlation(x)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "<decorator-gen-546>", line 24, in envelope_correlation
      File "/home/larsoner/python/mne-connectivity/mne_connectivity/envelope.py", line 167, in envelope_correlation
        triu_inds = np.triu_indices(n_nodes, k=0)
    UnboundLocalError: local variable 'n_nodes' referenced before assignment

When would this ever occur tho? Why would it be 0 epochs?

larsoner commented 3 years ago

Yes it happens. Epochs objects can be empty for example. I'm okay with case raising an error, but it should be explicit not cryptic like the current one

drammock commented 3 years ago

ok, for me the problems are resolved now, after re-writing the NetCDF objects while on this branch (last time I was reading on this branch some NetCDF objects written with main branch a couple days before)