Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: "channel_name" error when exporting from Spikeinterface back to the original NWB file #1135

Open
2 tasks done
borrepp opened this issue Nov 11, 2024 · 8 comments
Open
2 tasks done
Assignees
Labels

Comments

@borrepp
Copy link

borrepp commented Nov 11, 2024

What happened?

Hello everyone.

I have an NWB with ephys data (behavior & raw 30K data). I'm using spikeinterface to read from NWB for sorting the single neurons and to create the LFPs. Tha parts is done and works fine.

However, when I tried to go back to the NWB, I saw that "NEUROCONV" makes use of the spikeinterface.recording property "channel_name" to identify which electrodes are being imported.
This "channel_name" in the spikeinterface.recording corresponds/matches with the "nwb.electrodes.id". But this column name does not exist in the original NWB. Therefore, when "adding recording to nwbfile" calls "add_electrdes_to_nwbfile" is trying to append all the channels as "new electrodes" because the function "_get_electrodes_table_global_ids" is returning an empty list.

I think that might be better to FIRST check whether the nwbfile has the "channel_name" the electrodeTable (~line 580 in spikeinterface.py) before checking which electrodes to append (~line 568 in spikeinterface.py).

Hope it makes sense this comment.

Also, I'm including the error that I got when I run "add_electrodes_to_nwbfile" in the traceback section. However, I think this error is indirectly related to the comment I made above.

Steps to Reproduce

from pynwb import NWBHDF5IO
import neuroconv.tools.spikeinterface.spikeinterface as siconv

fPrepo_io = NWBHDF5IO(nwbPrepro_path, mode="r+")
nwbPrepo = fPrepo_io.read()

electrode_groups_names = [k for k in nwbPrepro.acquisition.keys() if 'raw-' in k]
print(electrode_groups_names)

print('Extract into SI the first electrode group: ', electrode_groups_names[0])

si_recording_raw = read_nwb(
        file_path=nwbPrepro_path, 
        electrical_series_path ='acquisition/' + electrode_groups_names[0],
        load_recording=True,
        load_sorting=False
)

si_recording_lfp = bandpass_filter(si_recording_raw, **bandpass_params)
si_recording_lfp1k = resample(si_recording_lfp, **resample_params)

siconv.add_electrodes_to_nwbfile(
    recording = si_recording_lfp1k,
    nwbfile = nwbPrepo)

Traceback

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[9], line 1
----> 1 siconv.add_electrodes_to_nwbfile(
      2     recording = si_recording_lfp1k,
      3     nwbfile = nwbPrepo)

File m:\Monkey_Python\SIenv\Lib\site-packages\neuroconv\tools\spikeinterface\spikeinterface.py:573, in add_electrodes_to_nwbfile(recording, nwbfile, metadata, exclude, null_values_for_properties)
    571     data_dict = {property: data_to_add[property]["data"][channel_index] for property in properties_with_data}
    572     electrode_kwargs.update(**data_dict)
--> 573     nwbfile.add_electrode(**electrode_kwargs, enforce_unique_id=True)
    575 # The channel_name column as we use channel_name, group_name as a unique identifier
    576 # We fill previously inexistent values with the electrode table ids
    577 electrode_table_size = len(nwbfile.electrodes.id[:])

File m:\Monkey_Python\SIenv\Lib\site-packages\hdmf\utils.py:668, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    666 def func_call(*args, **kwargs):
    667     pargs = _check_args(args, kwargs)
--> 668     return func(args[0], **pargs)

File m:\Monkey_Python\SIenv\Lib\site-packages\pynwb\file.py:728, in NWBFile.add_electrode(self, **kwargs)
    725     else:
    726         d.pop(col_name)  # remove args from d if not set
--> 728 self.electrodes.add_row(**d)

File m:\Monkey_Python\SIenv\Lib\site-packages\hdmf\utils.py:668, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    666 def func_call(*args, **kwargs):
    667     pargs = _check_args(args, kwargs)
--> 668     return func(args[0], **pargs)

File m:\Monkey_Python\SIenv\Lib\site-packages\hdmf\common\table.py:706, in DynamicTable.add_row(self, **kwargs)
    704     if row_id in self.id:
    705         raise ValueError("id %i already in the table" % row_id)
--> 706 self.id.append(row_id)
    708 for colname, colnum in self.__colids.items():
    709     if colname not in data:

File m:\Monkey_Python\SIenv\Lib\site-packages\hdmf\container.py:986, in Data.append(self, arg)
    984 def append(self, arg):
    985     self._validate_new_data_element(arg)
--> 986     self.__data = append_data(self.__data, arg)

File m:\Monkey_Python\SIenv\Lib\site-packages\hdmf\data_utils.py:36, in append_data(data, arg)
     34 shape = list(data.shape)
     35 shape[0] += 1
---> 36 data.resize(shape)
     37 data[-1] = arg
     38 return data

File m:\Monkey_Python\SIenv\Lib\site-packages\h5py\_hl\dataset.py:666, in Dataset.resize(self, size, axis)
    664 with phil:
    665     if self.chunks is None:
--> 666         raise TypeError("Only chunked datasets can be resized")
    668     if axis is not None:
    669         if not (axis >=0 and axis < self.id.rank):

TypeError: Only chunked datasets can be resized

Operating System

Windows

Python Executable

Python

Python Version

3.11

Package Versions

No response

Code of Conduct

@borrepp borrepp added the bug label Nov 11, 2024
@borrepp borrepp changed the title [Bug]: "channel_name" related error when exporting from Spikeinterface back to the original NWB file [Bug]: "channel_name" error when exporting from Spikeinterface back to the original NWB file Nov 11, 2024
@h-mayorquin
Copy link
Collaborator

Hi, thanks for opening an issue.

Can you give more details on what you are trying to achieve:

  • Why are you using the add_electrodes_to_nwbfile function by itself and what are you trying to achieve?
  • How did you generate the first NWBFile to begin with?

@borrepp
Copy link
Author

borrepp commented Nov 11, 2024

Hello Heberto @h-mayorquin

Thanks for your response.

I was testing to see if it would add more electrodes or recognize that they already exist. I wanted to track the process of adding the recording step by step to ensure that it was saved properly.

We have a custom code to create the original NWB file. I have run the nwb-inspector and it passes as a good nwb file.

When I added the recording directly, I also got the same error

Thanks in advance for your help

siconv.add_recording_to_nwbfile(
    recording = si_recording_lfp1k,
    nwbfile = nwbPrepo,
    write_as='lfp'
    )

Error:

{
 "name": "TypeError",
 "message": "Only chunked datasets can be resized",
 "stack": "---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[10], line 1
----> 1 siconv.add_recording_to_nwbfile(
   2     recording = si_recording_lfp1k,
   3     nwbfile = nwbPrepo,
   4     write_as='lfp'
   5     )

File m:\\Monkey_Python\\SIenv\\Lib\\site-packages\
euroconv\\tools\\spikeinterface\\spikeinterface.py:1127, in add_recording_to_nwbfile(recording, nwbfile, metadata, starting_time, write_as, es_key, write_electrical_series, write_scaled, compression, compression_opts, iterator_type, iterator_opts, always_write_timestamps)
1124 elif metadata is None:
1125     metadata = _get_nwb_metadata(recording=recording)
-> 1127 add_electrodes_info_to_nwbfile(recording=recording, nwbfile=nwbfile, metadata=metadata)
1129 if write_electrical_series:
1130     number_of_segments = recording.get_num_segments()

File m:\\Monkey_Python\\SIenv\\Lib\\site-packages\
euroconv\\tools\\spikeinterface\\spikeinterface.py:1008, in add_electrodes_info_to_nwbfile(recording, nwbfile, metadata)
1006 add_devices_to_nwbfile(nwbfile=nwbfile, metadata=metadata)
1007 add_electrode_groups_to_nwbfile(recording=recording, nwbfile=nwbfile, metadata=metadata)
-> 1008 add_electrodes_to_nwbfile(recording=recording, nwbfile=nwbfile, metadata=metadata)

File m:\\Monkey_Python\\SIenv\\Lib\\site-packages\
euroconv\\tools\\spikeinterface\\spikeinterface.py:573, in add_electrodes_to_nwbfile(recording, nwbfile, metadata, exclude, null_values_for_properties)
 571     data_dict = {property: data_to_add[property][\"data\"][channel_index] for property in properties_with_data}
 572     electrode_kwargs.update(**data_dict)
--> 573     nwbfile.add_electrode(**electrode_kwargs, enforce_unique_id=True)
 575 # The channel_name column as we use channel_name, group_name as a unique identifier
 576 # We fill previously inexistent values with the electrode table ids
 577 electrode_table_size = len(nwbfile.electrodes.id[:])

File m:\\Monkey_Python\\SIenv\\Lib\\site-packages\\hdmf\\utils.py:668, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
 666 def func_call(*args, **kwargs):
 667     pargs = _check_args(args, kwargs)
--> 668     return func(args[0], **pargs)

File m:\\Monkey_Python\\SIenv\\Lib\\site-packages\\pynwb\\file.py:728, in NWBFile.add_electrode(self, **kwargs)
 725     else:
 726         d.pop(col_name)  # remove args from d if not set
--> 728 self.electrodes.add_row(**d)

File m:\\Monkey_Python\\SIenv\\Lib\\site-packages\\hdmf\\utils.py:668, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
 666 def func_call(*args, **kwargs):
 667     pargs = _check_args(args, kwargs)
--> 668     return func(args[0], **pargs)

File m:\\Monkey_Python\\SIenv\\Lib\\site-packages\\hdmf\\common\\table.py:706, in DynamicTable.add_row(self, **kwargs)
 704     if row_id in self.id:
 705         raise ValueError(\"id %i already in the table\" % row_id)
--> 706 self.id.append(row_id)
 708 for colname, colnum in self.__colids.items():
 709     if colname not in data:

File m:\\Monkey_Python\\SIenv\\Lib\\site-packages\\hdmf\\container.py:986, in Data.append(self, arg)
 984 def append(self, arg):
 985     self._validate_new_data_element(arg)
--> 986     self.__data = append_data(self.__data, arg)

File m:\\Monkey_Python\\SIenv\\Lib\\site-packages\\hdmf\\data_utils.py:36, in append_data(data, arg)
  34 shape = list(data.shape)
  35 shape[0] += 1
---> 36 data.resize(shape)
  37 data[-1] = arg
  38 return data

File m:\\Monkey_Python\\SIenv\\Lib\\site-packages\\h5py\\_hl\\dataset.py:666, in Dataset.resize(self, size, axis)
 664 with phil:
 665     if self.chunks is None:
--> 666         raise TypeError(\"Only chunked datasets can be resized\")
 668     if axis is not None:
 669         if not (axis >=0 and axis < self.id.rank):

TypeError: Only chunked datasets can be resized"
}

@borrepp
Copy link
Author

borrepp commented Nov 11, 2024

I just want to confirm what I want to achieve:

What I want to achieve is to re-import the "lfp" data into the original NWB.

I will need to do the same ("reimporting") for the sorting results (i.e., sorting_analyzer).

Also, the way I have been able to sort this problem is by adding the "channel_name" column to the original nwbFile on my own. That allows me to reimport the spikeinterface.recording into the original nwbFile without a problem.

@h-mayorquin
Copy link
Collaborator

Thanks for the explanation.

I took a deeper look at this and found out that this is a problem with the non-expandable nature electrode table and not a problem with the function per see. We actually have a test for this for in-memory nwbfiles here:

def test_manual_row_adition_before_add_electrodes_function_to_nwbfile(self):
"""Add some rows to the electrode tables before using the add_electrodes_to_nwbfile function"""
values_dic = self.defaults
values_dic.update(id=123)
self.nwbfile.add_electrode(**values_dic)
values_dic.update(id=124)
self.nwbfile.add_electrode(**values_dic)
add_electrodes_to_nwbfile(recording=self.recording_1, nwbfile=self.nwbfile)
expected_ids = [123, 124, 2, 3, 4, 5]
expected_names = ["123", "124", "a", "b", "c", "d"]
self.assertListEqual(list(self.nwbfile.electrodes.id.data), expected_ids)
self.assertListEqual(list(self.nwbfile.electrodes["channel_name"].data), expected_names)
def test_manual_row_adition_after_add_electrodes_function_to_nwbfile(self):
"""Add some rows to the electrode table after using the add_electrodes_to_nwbfile function"""
add_electrodes_to_nwbfile(recording=self.recording_1, nwbfile=self.nwbfile)
values_dic = self.defaults
# Previous properties
values_dic.update(rel_x=0.0, rel_y=0.0, id=123, channel_name=str(123))
self.nwbfile.add_electrode(**values_dic)
values_dic.update(rel_x=0.0, rel_y=0.0, id=124, channel_name=str(124))
self.nwbfile.add_electrode(**values_dic)
values_dic.update(rel_x=0.0, rel_y=0.0, id=None, channel_name="6") # automatic ID set
self.nwbfile.add_electrode(**values_dic)
expected_ids = [0, 1, 2, 3, 123, 124, 6]
expected_names = ["a", "b", "c", "d", "123", "124", "6"]
self.assertListEqual(list(self.nwbfile.electrodes.id.data), expected_ids)
self.assertListEqual(list(self.nwbfile.electrodes["channel_name"].data), expected_names)
def test_manual_row_adition_before_add_electrodes_function_optional_columns_to_nwbfile(self):
"""Add some rows including optional columns to the electrode tables before using the add_electrodes_to_nwbfile function."""
values_dic = self.defaults
values_dic.update(id=123)
self.nwbfile.add_electrode(**values_dic, x=0.0, y=1.0, z=2.0)
values_dic.update(id=124)
self.nwbfile.add_electrode(**values_dic, x=1.0, y=2.0, z=3.0)
# recording_1 does not have x, y, z positions
add_electrodes_to_nwbfile(recording=self.recording_1, nwbfile=self.nwbfile)
expected_ids = [123, 124, 2, 3, 4, 5]
expected_x = [0, 1, np.nan, np.nan, np.nan, np.nan]
expected_y = [1, 2, np.nan, np.nan, np.nan, np.nan]
expected_z = [2, 3, np.nan, np.nan, np.nan, np.nan]
self.assertListEqual(list(self.nwbfile.electrodes.id.data), expected_ids)
self.assertListEqual(list(self.nwbfile.electrodes["x"].data), expected_x)
self.assertListEqual(list(self.nwbfile.electrodes["y"].data), expected_y)
self.assertListEqual(list(self.nwbfile.electrodes["z"].data), expected_z)

The problem is when you write the file then the electrode table is no longer expandable.

I will discuss this with the team and will comeback to you with a plan for changing this or a workaround.

@h-mayorquin h-mayorquin self-assigned this Nov 12, 2024
@h-mayorquin
Copy link
Collaborator

Hi, we discussed this with the team today. As I mentioned, the problem is that the electrodes table is not expandable in your first set. We are working on a solution here:

#1137

Once this is done I can show you how to add some lines to the function where you wrote the nwbfile so your electrode table is expandable.

Making objects expandable is something have been discussed for a long time. See some references here:
hdmf-dev/hdmf#1153
hdmf-dev/hdmf#1017
NeurodataWithoutBorders/pynwb#1067
NeurodataWithoutBorders/pynwb#918
hdmf-dev/hdmf#1093

@borrepp
Copy link
Author

borrepp commented Nov 12, 2024

Hello Heberto @h-mayorquin

Thank you so much for all the information.

I understood that the error was that the electrode table was not expandable. However, this error shouldn't occur because I'm using the "add_recording_to_nwbfile" with a "spikeinterface.recording" input extracted from the same nwbFile. Does it make sense what I mean?

I have an NWBfile that has a raw signal. I import the raw signal with spikeInterface, I do filtering (or some preprocessing like motion correction, etc). Then, I want to save the recording back into the NWBfile. The Neuroconv function should not append new electrodes. It should have recognized the channel names & group names.

My first comment pointed out that when adding electrodes, a step is always performed when adding a recording or sorting analyzer, the function should add the column "channel_name" to the NWBfile first and later check the recording to find which electrodes to append.

The reason is that the column: "channel_name" is not a default/requisite column in the electrode's NWB scheme. Therefore, when it searches for that column, the function "_get_electrodes_table_global_ids" returns empty channels.

In fact, by adding the column "channel_name" to the NWBfile's electrode table, there is not error when I add the recording object because it recognizes that the channels are already in the electrode table.

I hope what I'm trying to explain makes sense.

Thanks.
Best
Pepe

@h-mayorquin
Copy link
Collaborator

Ok, I think I understand what you are proposing. You are saying that we should add the channel name before determining the global indices to decide whether we append electrodes or not. I think this makes sense in your case but I am not sure can be done safely in general:

Consider the case when the recordings do not match. We start appending the channel column but then the group from the recording is in fact different from the one in the table. In that case, you will be appending both the column of the ids as channel nam and then appending all the channels as rows which is wrong.

There could be a solution to where we check the pair of the group in the electrode table together with the channel id but our function is a little bit too complex already and I am not sure we should add yet another corner case here given that you already have a simple solution (just add channel name to your original script for writing the nwbfile).

Can you send me an email to h . mayorquin in gmail (no spaces) so we can have a quick discussion about the motivation. Maybe it makes sense but I want to understand more about your usage.

@borrepp
Copy link
Author

borrepp commented Nov 12, 2024

Sure, I will email you now.

Thank you so much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants