Skip to content

Conversation

@calderast
Copy link
Owner

@calderast calderast commented Sep 17, 2025

Spyglass ignores ElectricalSeries conversion factors and just assumes everything is in uV. I will submit a spyglass PR to edit this, but just storing ours in uV is a reasonable workaround for now.

P.S. I am writing this in the airport on the way to Berlin, so I didn't test this even a little bit. But I wanted to submit it rn so people aren't blocked. Sorry to my fans (and haters) if this breaks anything

@calderast calderast self-assigned this Sep 17, 2025
@calderast calderast added ephys All things related to electrophysiology processing quick-fix Minor one-liner to improve spyglass For NWB compatibility with Loren Frank Lab's spyglass https://github.com/LorenFrankLab/spyglass labels Sep 17, 2025
@rly
Copy link
Collaborator

rly commented Sep 17, 2025

Also see discussion in LorenFrankLab/trodes_to_nwb#140

FYI this goes against the NWB standard where raw array data * global conversion factor * channel conversion factor should be stored in volts. https://github.com/NeurodataWithoutBorders/nwb-schema/blob/dev/core/nwb.ecephys.yaml#L36 . So PyNWB does not let you pass unit="microvolts" to ElectricalSeries.

Also if the array data are stored in microvolts, then the conversion factor (or channel conversion factor) should then be 1e6.

@calderast
Copy link
Owner Author

Also see discussion in LorenFrankLab/trodes_to_nwb#140

FYI this goes against the NWB standard where raw array data * global conversion factor * channel conversion factor should be stored in volts. https://github.com/NeurodataWithoutBorders/nwb-schema/blob/dev/core/nwb.ecephys.yaml#L36 . So PyNWB does not let you pass unit="microvolts" to ElectricalSeries.

Also if the array data are stored in microvolts, then the conversion factor (or channel conversion factor) should then be 1e6.

Got it, I’ll change that to be a conversion factor to V - FWIW, I think the current way we store things (raw data and conversion factor, consistent with the NWB standard) is preferable, and I agree with all of Kyu’s points in the spyglass discussion. But I am trying to get better at choosing the path of least resistance re spyglass… so I will begrudgingly carry on 🥲

@rly
Copy link
Collaborator

rly commented Sep 18, 2025

Got it, I’ll change that to be a conversion factor to V - FWIW, I think the current way we store things (raw data and conversion factor, consistent with the NWB standard) is preferable, and I agree with all of Kyu’s points in the spyglass discussion. But I am trying to get better at choosing the path of least resistance re spyglass… so I will begrudgingly carry on 🥲

Sounds good! The ultimate goal of all this is to do science, so yeah sometimes the quick fixes make the most sense.

Have a good trip!

@calderast
Copy link
Owner Author

@rly - by converting to uV I also end up changing the datatype from int16 to float32, which is waaaay bigger - what do you think about this? Now that I actually think about it I feel that doubling the size of already big data is not wise ... :(

@calderast calderast changed the title Store ElectricalSeries in uV, rename electrode table impedance column to imp Store ElectricalSeries in uV for spyglass compatability Oct 8, 2025
@rly
Copy link
Collaborator

rly commented Oct 9, 2025

Sorry for my delay in response. If I remember correctly, the Frank Lab data are stored in uV as int16. The few datasets that I have looked at have their raw electricalseries data as int16. I think if you are storing as uV, rounding/truncating will not have a significant impact. Non-zero, but probably insignificant.

@rly
Copy link
Collaborator

rly commented Oct 9, 2025

See also LorenFrankLab/rec_to_nwb#54

@rly
Copy link
Collaborator

rly commented Oct 9, 2025

@calderast
Copy link
Owner Author

calderast commented Oct 27, 2025

Left this PR hanging for a while because I thought we had maybe fixed things on the spyglass end. I am not 100% sure I'm correct anymore on that front, so going to move forward with this change. I think we can safely accept the loss of precision that comes from storing our raw ephys data in uV as int16, as the Frank Lab makes the same choice and the noise level of our recordings makes a sub-uV level of precision unnecessary.

Note that I am making this choice for spyglass compatibility (as ElectricalSeries conversion factors are often ignored when they shouldn't be, as Frank Lab stores all of their data in uV so spyglass assumes this). The NWB standard is to store data in ADC units with conversion factor to V, which I would prefer.

@calderast calderast merged commit 90b6782 into main Oct 27, 2025
7 checks passed
@calderast calderast deleted the update-conversion branch October 27, 2025 18:11
@calderast calderast removed the quick-fix Minor one-liner to improve label Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ephys All things related to electrophysiology processing spyglass For NWB compatibility with Loren Frank Lab's spyglass https://github.com/LorenFrankLab/spyglass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants