Record real electrode ids in broadband HDF5 output - #193
Open
namthor9 wants to merge 1 commit into
Open
Conversation
detect_stream_parameters reported channels as range(len(frame_data)) and the writer copied that straight into general/extracellular_ephys/electrodes/id, so every row was labeled by its position in the frame. A recording therefore carried no way to tell which electrode a row came from, and any configuration whose electrodes aren't 0..N-1 in ascending order — a sparse selection, or one deliberately ordered by the user — was silently mislabeled all the way through analysis. The new synapse.utils.electrode_ids reconstructs that identity. Frames describe their own layout in channel_ranges (a type, a count, and the logical channel ids in frame order), and the logical -> physical electrode mapping lives in the applied device configuration, which build_channel_to_electrode_map reads out of DeviceInfo. derive_electrode_row_ids combines the two into one id per frame_data entry, re-keying GPIO rows into a high namespace so ids stay unique within the table. Identity degrades explicitly rather than quietly: the electrode map is applied only when every electrode row resolves, so a dataset never mixes the two id spaces, and frames with no usable channel_ranges fall back to the old positional scheme with a warning printed to the console. Which of the three happened is recorded as the electrodes group's id_source attribute, and the logical channel id and channel type are written alongside id as equal-length datasets so downstream analysis can reconstruct the full mapping. The live plotter picks up the same ids, so its channel labels now name real electrodes. The offline plotter labeled its DataFrame columns positionally while selecting them by channel id, so --channels picked the wrong traces for any recording whose ids aren't 0..N-1; it now labels columns by channel id and reports id_source alongside the channel count. This mirrors deriveElectrodeRowIds in nexus-desktop, which writes the same layout from the desktop recording path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the
synapsectlrecording half of the electrode channel mapping bug Horacio reported. With a Nixel512 config of 5 ground-referenced channels on electrodes6, 16, 10, 14, 20, a recording came out labeled0, 1, 2, 3, 4— no way to tell which electrode any row came from.Companion to sciencecorp/nexus-desktop#416, which writes the same HDF5 layout from the desktop recording path, and sciencecorp/headstage#527, the firmware fix that makes the stream report configuration order.
The problem
detect_stream_parametersreported channels asrange(len(frame_data))and the writer copied that straight intogeneral/extracellular_ephys/electrodes/id, so every row was labeled by its position in the frame. A recording therefore carried no way to tell which electrode a row came from, and any configuration whose electrodes aren't0..N-1in ascending order — a sparse selection, or one deliberately ordered by the user — was silently mislabeled all the way through analysis.The fix
The new
synapse.utils.electrode_idsreconstructs that identity. Frames describe their own layout inchannel_ranges(a type, a count, and the logical channel ids in frame order), and the logical → physical electrode mapping lives in the applied device configuration, whichbuild_channel_to_electrode_mapreads out ofDeviceInfo.derive_electrode_row_idscombines the two into one id perframe_dataentry, re-keying GPIO rows into a high namespace so ids stay unique within the table.Identity degrades explicitly rather than quietly:
id_sourceelectrode_mapchannel_rangespositionalThe electrode map is applied only when every electrode row resolves, so a dataset never mixes the two id spaces, and frames with no usable
channel_rangesfall back to positional with a warning printed to the console. Which level applied is recorded as the electrodes group'sid_sourceattribute, and the logical channel id and channel type are written alongsideidas equal-length datasets so downstream analysis can reconstruct the full mapping.device.info()moves ahead of stream detection so the map is available before the first frame is parsed. The live plotter is fed the same ids, so its channel labels now name real electrodes.Also fixed:
--channelsselected the wrong tracesSeparate pre-existing bug in the offline plotter, surfaced by this work. It labeled its DataFrame columns positionally (
columns=range(number_of_channels)) butfilter_channelsselects by label (data.loc[:, channel_ids]), so--channelspicked the wrong traces for any recording whose ids aren't0..N-1— and would have raisedKeyErroron every recording written by this PR. Columns are now labeled by channel id, and the plotter reportsid_sourcealongside the channel count.Testing
Full suite passes (153). 16 new tests: 13 unit tests over the derivation and map building, plus 3 that run the writer and read the HDF5 back to assert the on-disk layout. Coverage includes Horacio's exact
6, 16, 10, 14, 20config asserting the recorded order is not sorted, each degradation path, the placeholder-electrode peripheral skip, and a GPIO frame wherelen(electrodes/id)must still equal the per-frame entry count.Not yet verified against hardware — until headstage#527 lands, a Nixel512 stream takes the
channel_ranges omit channel_idspath and records logical ids with a warning.