Skip to content

Record real electrode ids in broadband HDF5 output - #193

Open
namthor9 wants to merge 1 commit into
mainfrom
maxr/fix-electrode-channel-mapping
Open

Record real electrode ids in broadband HDF5 output#193
namthor9 wants to merge 1 commit into
mainfrom
maxr/fix-electrode-channel-mapping

Conversation

@namthor9

Copy link
Copy Markdown
Contributor

Fixes the synapsectl recording half of the electrode channel mapping bug Horacio reported. With a Nixel512 config of 5 ground-referenced channels on electrodes 6, 16, 10, 14, 20, a recording came out labeled 0, 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_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 fix

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:

id_source meaning
electrode_map real physical electrode ids
channel_ranges device-reported logical channel ids
positional row index — the old behavior, no identity at all

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 positional with a warning printed to the console. Which level applied 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.

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: --channels selected the wrong traces

Separate pre-existing bug in the offline plotter, surfaced by this work. It labeled its DataFrame columns positionally (columns=range(number_of_channels)) but filter_channels selects by label (data.loc[:, channel_ids]), so --channels picked the wrong traces for any recording whose ids aren't 0..N-1 — and would have raised KeyError on every recording written by this PR. Columns are now labeled by channel id, and the plotter reports id_source alongside 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, 20 config asserting the recorded order is not sorted, each degradation path, the placeholder-electrode peripheral skip, and a GPIO frame where len(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_ids path and records logical ids with a warning.

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.
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.

1 participant