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 Check]: ElectricalSeries of dtype int16 with conversion=1.0 and offset=0.0 is potentially wrong? #395

Open
2 tasks done
CodyCBakerPhD opened this issue Aug 8, 2023 · 4 comments · May be fixed by #408
Open
2 tasks done
Assignees
Labels
category: new check a new best practices check to apply to all NWBFiles and their contents priority: low alternative solution already working and/or relevant to only specific user(s)

Comments

@CodyCBakerPhD
Copy link
Contributor

What would you like to see added to the NWBInspector?

More of a question for either @alejoe91 or @samuelgarcia based on their experience

If you saw an ElectricalSeries that was of dtype uint16 or int16 and the conversion factor (spikeinterface 'gain_to_uV') was all 1.0 and offset 0.0, this is most likely incorrect, right?

If so, how strongly incorrect is it?

@bendichter @rly @oruebel Would/Should it technically count as being invalid? Should it prevent DANDI upload?

Pseudocode or other logic of the check

No response

Do you have any interest in helping implement the check function?

Yes.

Code of Conduct

@CodyCBakerPhD CodyCBakerPhD added the category: new check a new best practices check to apply to all NWBFiles and their contents label Aug 8, 2023
@rly
Copy link
Contributor

rly commented Aug 8, 2023

Good question. I think it is highly unlikely to be correct because I cannot think of a situation where electrodes in a neurophysiology experiment would be recording in integer volts. I think that should be invalid. Not sure about preventing DANDI upload. We could err on a stricter rule and relax it if an unexpected use case comes to our attention.

@bendichter
Copy link
Contributor

I agree, int data with a conversion of 1.0 is likely an error.

@samuelgarcia
Copy link

Hi Cody. This seams an error. Where this data is from ?

@CodyCBakerPhD
Copy link
Contributor Author

@samuelgarcia Default TDT and SpikeGadgets v1 both default to 'placeholder' gains in neo/SI

For the GUIDE support for those, we're therefore making the gains a required input as per this discussion

@alessandratrapani alessandratrapani linked a pull request Sep 27, 2023 that will close this issue
4 tasks
@stephprince stephprince added the priority: low alternative solution already working and/or relevant to only specific user(s) label Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: new check a new best practices check to apply to all NWBFiles and their contents priority: low alternative solution already working and/or relevant to only specific user(s)
Projects
None yet
6 participants