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

Add electrodes interface for Huszar #65

Merged
merged 10 commits into from
Jun 28, 2023

Conversation

h-mayorquin
Copy link
Contributor

This will add electrode information even when the Neuroscope interface for the raw data is no present.


# This should transform to microvolts which is the standard in spikeinterface
recording.set_channel_gains(channel_ids=channel_ids, gains=np.ones(num_channels) * 0.195)
recording.set_channel_offsets(channel_ids=channel_ids, offsets=np.ones(num_channels) * 32_768)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa whoa, do offsets get set to the real extractor? Or to a corresponding intan reading of the info.rhd?

Thing is, this is signed data - usually that offset is only for unsigned intan data

I could have sworn that when using SI to read this stuff the offset is only set if its unsigned

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing that out. I am losing a multiplicative factor. Look at how the oficial implementation for reading intan that intan systems provides looks:

https://github.com/Intan-Technologies/load-rhd-notebook-python/blob/9026c8c0ac0693ed4896c2f50f55a3f57237bb68/importrhdutilities.py#L579

But I think they use the offset there is because they initially define the data as unsigned:
https://github.com/Intan-Technologies/load-rhd-notebook-python/blob/9026c8c0ac0693ed4896c2f50f55a3f57237bb68/importrhdutilities.py#L511

Which sounds like what are you saying as Neuroscope already loads the data signed and then has the offset 0.
https://github.com/NeuralEnsemble/python-neo/blob/f608309c5ce031ecd905349c140b07a3dafb057d/neo/rawio/neuroscoperawio.py#L111

I have become very suspicious of that Neuroscope reader but if indeed we are going to read the data using it, we should probably set the offest to 0.

The other readers for intan have the offset though:

pyintan has it though:
https://github.com/alejoe91/pyintan/blob/92ee10acad13bd2b5207a158a2e38ce222be716a/pyintan/intan.py#LL108C19-L108C19

And the reader for intan in neo:
https://github.com/NeuralEnsemble/python-neo/blob/f608309c5ce031ecd905349c140b07a3dafb057d/neo/rawio/intanrawio.py#L491

Copy link
Member

@CodyCBakerPhD CodyCBakerPhD Jun 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the base readers underlying pyintan and neo always set it, my contention was that something in the SI loader would make the decision whether or not to set that to the extractor, somewhere around https://github.com/SpikeInterface/spikeinterface/blob/main/src/spikeinterface/extractors/neoextractors/neobaseextractor.py#L156

At least the last time I used the intan reader on data, that's how it worked (I've ran before on both signed and unsigned intan data, but that about a year ago) - point being, the offset was only needed when the data was unsigned

Anyway, yes for Neuroscope it should soundly be zero, since it was acquired as signed intan (which also should not use offsets, which if SI currently does is, I suspect, a bug)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I commented out the line because we are going to use Neuroscope. I also tested Neuroscope for the valero data and the offset is zero.

Beside that,
Are you saying that using SI to read intan data should set the offsets to zero? Not clear to me why and it is not the case if I read Valero data with Intan:

image

I don't really understand what do you mean by "acquiring with signed intan", can you elaborate?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm saying years ago I've had to convert data from Intan .rhd format for at least two separate projects - in one of them, the dtype was int16 and the other was uint16, according to the SI readers at the time, which I trusted to be accurate in their parsing of the header information - I don't really know more beyond that, just recounting that I've seen both, and offset from SI was only non-zero for the unsigned

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got you, yeah, I am confused about this. Pyintan has this which indicates that there are some versions where the data is typed:

https://github.com/NeuralEnsemble/python-neo/blob/f608309c5ce031ecd905349c140b07a3dafb057d/neo/rawio/intanrawio.py#L473-L491

But I can't find any references to it in the oficial repo provided by the company...

@garrettmflynn garrettmflynn changed the base branch from master to add-huszar-metadata June 27, 2023 20:33
@garrettmflynn
Copy link
Contributor

@CodyCBakerPhD Uploaded a stub file with preliminary ecephys metadata to DANDI for this project: https://dandiarchive.org/dandiset/000552?pos=1

@h-mayorquin
Copy link
Contributor Author

@CodyCBakerPhD Uploaded a stub file with preliminary ecephys metadata to DANDI for this project: https://dandiarchive.org/dandiset/000552?pos=1

Did this work?

@CodyCBakerPhD
Copy link
Member

Did this work?

I'm having him try not using the ElectrodeInterface and use the new CellExplorerSortingInterface from NeuroConv to see what that looks like

Even after fixing the group indexing issue, Garretts latest file still had that channel name issue from before

@garrettmflynn
Copy link
Contributor

A new file is uploaded to DANDI, which I've confirmed has the appropriate Ecephys metadata structure

@CodyCBakerPhD CodyCBakerPhD merged commit 3450410 into add-huszar-metadata Jun 28, 2023
1 check passed
@CodyCBakerPhD CodyCBakerPhD deleted the huzsar_electrode_metadata branch June 28, 2023 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants