From 904e0ff6b13f602e1dae9618aa1ccd5662981841 Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Tue, 2 Jun 2026 18:38:52 -0400 Subject: [PATCH 1/4] Ignore .codegraph --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 3a1f1fba..d1606680 100644 --- a/.gitignore +++ b/.gitignore @@ -54,4 +54,5 @@ Release/ upstream */uv.lock -.test_cache/ \ No newline at end of file +.test_cache/ +.codegraph/ \ No newline at end of file From a017c2b7b1d58f12a18a6532b0f478da53ddac68 Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Tue, 2 Jun 2026 21:55:30 -0400 Subject: [PATCH 2/4] =?UTF-8?q?CMP:=20match=20by=20(bank,=20term),=20heade?= =?UTF-8?q?r-driven=20columns,=20positions=20in=20=C2=B5m?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rework how Blackrock .cmp channel-map files are applied to a device's in-memory channel config. Matching: rows are now matched to live channels by the device's own (bank, term) read from chaninfo, instead of by an ordinal channel ID (the old sort-and-assign start_chan+N scheme). start_chan shifts the CMP's bank letters by start_chan/32 banks to target a headstage's banks (129 -> +4 -> bank A maps to device bank E), which makes (bank, term) a globally-unique join key. This is robust to incomplete CMPs. position[]: the four cbPKT_CHANINFO.position slots now carry real geometry {x, y, size, headstage_id} instead of {col, row, bank, elec}. Columns: the //-comment header line immediately before the data (when present) is the ground truth for column order, matched by name (col/c, row/r, bank/b, elec/e, size/s, label/l); no header falls back to the legacy "col row bank electrode [label]" order, so existing files parse unchanged. Units: with no size column and a unit-spaced index grid (every non-zero col/row delta is exactly 1), values are electrode indices -> size 1 and x/y/size scaled by the 400 um Utah-array pitch. With a size column, or non-uniform spacing, values are taken at face value. Labels: stored verbatim; the hs{N}- prefix is dropped since the stored headstage id disambiguates reused labels. pycbsdk/cmp.py is kept in sync with the C++ parser; session.py docstrings corrected. C++ and Python unit tests updated; the hardware-gated nplay integration tests join device readback to parser output via ChanInfoField.BANK/TERM. Co-Authored-By: Claude Opus 4.8 (1M context) --- pycbsdk/src/pycbsdk/cmp.py | 260 +++++++++++++++++++--------- pycbsdk/src/pycbsdk/session.py | 28 +-- pycbsdk/tests/test_cmp.py | 156 +++++++++++++---- pycbsdk/tests/test_configuration.py | 151 +++++++++------- src/cbsdk/src/cmp_parser.cpp | 172 ++++++++++++++---- src/cbsdk/src/cmp_parser.h | 92 +++++++--- src/cbsdk/src/sdk_session.cpp | 78 ++++++--- tests/unit/test_cmp_parser.cpp | 201 +++++++++++++++++---- 8 files changed, 820 insertions(+), 318 deletions(-) diff --git a/pycbsdk/src/pycbsdk/cmp.py b/pycbsdk/src/pycbsdk/cmp.py index 7d272f18..a011d5db 100644 --- a/pycbsdk/src/pycbsdk/cmp.py +++ b/pycbsdk/src/pycbsdk/cmp.py @@ -1,25 +1,39 @@ """Parse and apply Blackrock ``.cmp`` channel-map files. -A ``.cmp`` file describes one headstage's electrode layout. Each data row -has the form:: - - col row bank electrode [label] - -where ``bank`` is a letter (``A``-``H``) and ``electrode`` is 1-based within -the bank. Rows are not guaranteed to be in channel order; this module sorts -by ``(bank, electrode)`` and assigns each sorted row an absolute 1-based -channel ID starting at a caller-supplied ``start_chan``. - -Labels commonly collide across headstages (every file may use -``elec1-1`` … ``elec1-128``), so each label is prefixed with ``"hs{hs_id}-"``. -Pass ``hs_id=0`` (the default) to skip prefixing — appropriate for -single-headstage rigs where the original labels are already unique. +A ``.cmp`` file describes one headstage's electrode layout. A data row holds +``col row bank electrode [size] [label]``. When a column-header comment line +immediately precedes the data (e.g. ``//col row bank elec label`` or +``//col row bank elec size label``) it is the ground truth for column order; +names are matched case-insensitively (col/c, row/r, bank/b, elec/e, size/s, +label/l), so the order is not assumed. With no header the legacy positional +order ``col row bank electrode [label]`` is used. + +Units: when no size column is supplied and the col/row values form a +unit-spaced index grid (every non-zero delta among the distinct col/row values +is exactly 1 — the manufacturer default), they are interpreted as electrode +indices: ``size`` becomes 1 and x/y/size are scaled by the 400 µm Utah-array +electrode pitch. Otherwise (a size column is present, or the spacing is +non-uniform) col/row/size are taken at face value. + +Entries are keyed by device ``(bank, term)`` — the same join key the C++ SDK +uses to match rows to live channels — not by an ordinal channel ID. Each +row's bank letter is shifted by ``start_chan // 32`` banks so a CMP can target +a specific headstage's banks (``start_chan=129`` → ``+4`` banks → CMP bank A +maps to device bank E). + +Labels are stored verbatim. ``hs_id`` identifies the headstage and is stored +in each entry's ``headstage`` field (it is **not** mixed into the label — the +stored headstage id already disambiguates labels reused across headstages). + +This module mirrors the C++ parser in ``src/cbsdk/src/cmp_parser.{h,cpp}``; the +device is configured through that C++ path (``Session.load_channel_map`` hands +it the file path), so this module is for inspection/CLI use. Typical use:: entries = parse_cmp("/path/to/headstage1.cmp", start_chan=1, hs_id=1) - for chan_id, entry in sorted(entries.items()): - print(chan_id, entry.label, entry.position) + for (bank, term), entry in sorted(entries.items()): + print(bank, term, entry.label, entry.x, entry.y) To apply CMPs to a live session, prefer :meth:`pycbsdk.Session.load_channel_map`. @@ -38,78 +52,166 @@ @dataclass(frozen=True) class CmpEntry: - """One parsed CMP row, ready to apply to an absolute channel.""" + """One parsed CMP row. - chan_id: int # 1-based absolute channel - position: tuple[int, int, int, int] # (col, row, bank_idx, electrode) - label: str # prefixed (e.g. "hs1-chan3") + The geometry fields populate ``cbPKT_CHANINFO.position[0..3]``; the device + ``(bank, term)`` are the keys used to find which channel the row applies to. + """ + + x: int # electrode x (µm for index grids, else face value) → position[0] + y: int # electrode y (µm for index grids, else face value) → position[1] + size: int # electrode size, same units as x/y (0 = unspecified) → position[2] + headstage: int # 1-based headstage id (== hs_id; 0 = none) → position[3] + bank: int # 1-based device bank (CMP bank + start_chan // 32); match key + term: int # 1-based terminal within bank (CMP electrode); match key + label: str # verbatim label (e.g. "chan12") + + +# Blackrock Utah arrays have 400 µm inter-electrode spacing; default index grids +# are scaled by this to convert electrode indices to micrometers. +_PITCH_UM = 400 + +_COLUMN_ALIASES = { + "col": "col", + "column": "col", + "c": "col", + "x": "col", + "row": "row", + "r": "row", + "y": "row", + "bank": "bank", + "b": "bank", + "elec": "elec", + "electrode": "elec", + "e": "elec", + "term": "elec", + "terminal": "elec", + "size": "size", + "s": "size", + "sz": "size", + "label": "label", + "l": "label", + "name": "label", +} + + +def _classify_column(tok: str) -> str: + """Map a header token to a column kind (or ``"unknown"``).""" + return _COLUMN_ALIASES.get(tok.strip().strip("',\"").lower(), "unknown") + + +def _resolve_columns(header: str | None) -> tuple[list[str], bool]: + """Resolve column order from a header comment, else the legacy order. + + Returns (columns, has_size). + """ + if header: + cols = [_classify_column(t) for t in header.split()] + if {"col", "row", "bank", "elec"} <= set(cols): + return cols, ("size" in cols) + return ["col", "row", "bank", "elec", "label"], False + + +def _parse_row(tokens: list[str], columns: list[str]) -> dict | None: + """Parse one data row per ``columns``. Returns None to skip a bad row.""" + out = {"col": None, "row": None, "bank": None, "elec": None, "size": 0, "label": ""} + for kind, tok in zip(columns, tokens): + if kind in ("col", "row", "elec", "size"): + try: + out[kind] = int(tok) + except ValueError: + return None + elif kind == "bank": + if not tok[:1].isalpha(): + return None + out["bank"] = ord(tok[0].upper()) - ord("A") + 1 + elif kind == "label": + out["label"] = tok + # unknown: ignore + if None in (out["col"], out["row"], out["bank"], out["elec"]): + return None + return out + + +def _is_unit_spaced_grid(raw: list[dict]) -> bool: + """True if every non-zero delta among distinct col and row values is 1.""" + deltas: set[int] = set() + for key in ("col", "row"): + vals = sorted({r[key] for r in raw}) + deltas.update(b - a for a, b in zip(vals, vals[1:])) + return deltas == {1} def parse_cmp( filepath: str | Path, start_chan: int = 1, hs_id: int = 0, -) -> dict[int, CmpEntry]: - """Parse a single CMP file and assign absolute channel IDs. +) -> dict[tuple[int, int], CmpEntry]: + """Parse a single CMP file into entries keyed by device ``(bank, term)``. Args: filepath: Path to the ``.cmp`` file. - start_chan: 1-based channel assigned to the first sorted row. - hs_id: Headstage identifier; labels are prefixed ``"hs{hs_id}-"``. - Pass ``0`` (the default) to leave labels un-prefixed. + start_chan: 1-based channel selecting the target banks; the CMP's bank + letters are shifted by ``start_chan // 32`` banks (1 → banks A…, + 129 → banks E…). + hs_id: Headstage identifier stored in each entry's ``headstage`` field + (0 = none). Does not affect labels. Returns: - Dict mapping absolute 1-based ``chan_id`` → :class:`CmpEntry`. + Dict mapping ``(bank, term)`` → :class:`CmpEntry`. Raises: - ValueError: If the file is malformed or contains no valid rows. + ValueError: If the file contains no valid rows. FileNotFoundError: If the file does not exist. """ - raw: list[tuple[int, int, int, int, str]] = [] # (col, row, bank_idx, elec, label) - description: str | None = None + raw: list[dict] = [] + description_seen = False + header_candidate: str | None = None + columns: list[str] | None = None + has_size = False with open(filepath) as fh: for line in fh: stripped = line.strip() - if not stripped or stripped.startswith("//"): + if not stripped: continue - if description is None: - description = stripped + if stripped.startswith("//"): + # A comment after the description may be the column header. + if description_seen: + header_candidate = stripped[2:] continue - parts = stripped.split() - if len(parts) < 4: - raise ValueError( - f"{filepath}: malformed row (need 'col row bank elec [label]'): " - f"{line.rstrip()!r}" - ) - bank = parts[2] - if len(bank) != 1 or not bank[0].isalpha(): - raise ValueError(f"{filepath}: invalid bank letter {bank!r}") - bank_idx = ord(bank[0].upper()) - ord("A") + 1 - raw.append( - ( - int(parts[0]), - int(parts[1]), - bank_idx, - int(parts[3]), - parts[4] if len(parts) > 4 else "", - ) - ) + if not description_seen: + description_seen = True + header_candidate = None + continue + # First data row: lock in the column order. + if columns is None: + columns, has_size = _resolve_columns(header_candidate) + entry = _parse_row(stripped.split(), columns) + if entry is not None: + raw.append(entry) if not raw: raise ValueError(f"{filepath}: no valid entries") - raw.sort(key=lambda r: (r[2], r[3])) - - # hs_id == 0 means "single headstage, no prefix needed". - prefix = "" if hs_id == 0 else f"hs{hs_id}-" - entries: dict[int, CmpEntry] = {} - for i, (col, row, bank_idx, elec, label) in enumerate(raw): - chan_id = start_chan + i - entries[chan_id] = CmpEntry( - chan_id=chan_id, - position=(col, row, bank_idx, elec), - label=f"{prefix}{label}", + # No size column + a unit-spaced index grid → electrode indices: size is 1 + # and x/y/size scale by the 400 µm pitch. Otherwise take values at face value. + scale = not has_size and _is_unit_spaced_grid(raw) + factor = _PITCH_UM if scale else 1 + + bank_offset = start_chan // 32 + entries: dict[tuple[int, int], CmpEntry] = {} + for r in raw: + dev_bank = r["bank"] + bank_offset + term = r["elec"] + entries[(dev_bank, term)] = CmpEntry( + x=r["col"] * factor, + y=r["row"] * factor, + size=_PITCH_UM if scale else (r["size"] if has_size else 0), + headstage=hs_id, + bank=dev_bank, + term=term, + label=r["label"], ) return entries @@ -122,9 +224,9 @@ def parse_cmp( def _parse_spec(spec: str) -> tuple[Path, int, int]: """Parse a ``FILE:START_CHAN:HS_ID`` or ``FILE:START_CHAN`` spec. - ``HS_ID`` defaults to ``0`` (no label prefix) when omitted; ``START_CHAN`` - defaults to ``1``. The CLI does not auto-stack headstages — callers must - pass each spec explicitly. + ``HS_ID`` defaults to ``0`` when omitted; ``START_CHAN`` defaults to ``1``. + The CLI does not auto-stack headstages — callers must pass each spec + explicitly. """ parts = spec.split(":") if not parts or not parts[0]: @@ -142,27 +244,26 @@ def _parse_spec(spec: str) -> tuple[Path, int, int]: def _dump(specs: list[tuple[Path, int, int]]) -> None: - """Parse each spec and print its entries, checking for chan_id collisions.""" - seen: dict[int, tuple[Path, str]] = {} + """Parse each spec and print its entries, checking for (bank, term) collisions.""" + seen: dict[tuple[int, int], tuple[Path, str]] = {} for path, start_chan, hs_id in specs: entries = parse_cmp(path, start_chan=start_chan, hs_id=hs_id) print( f"# {path} start_chan={start_chan} hs_id={hs_id} ({len(entries)} chans)" ) - for chan_id in sorted(entries): - e = entries[chan_id] - col, row, bank_idx, elec = e.position - if chan_id in seen: - prev_path, prev_label = seen[chan_id] + for key in sorted(entries): + e = entries[key] + if key in seen: + prev_path, prev_label = seen[key] print( - f" # WARNING: chan {chan_id} already claimed by " + f" # WARNING: (bank {e.bank}, term {e.term}) already claimed by " f"{prev_path.name} as {prev_label!r}", file=sys.stderr, ) - seen[chan_id] = (path, e.label) + seen[key] = (path, e.label) print( - f" chan {chan_id:>3} {e.label:<16s} col={col} row={row} " - f"bank={bank_idx} elec={elec}" + f" bank={e.bank} term={e.term:>2} {e.label:<16s} " + f"x={e.x} y={e.y} size={e.size} hs={e.headstage}" ) @@ -197,7 +298,7 @@ def main(argv: list[str] | None = None) -> int: description=( "Parse or apply Blackrock .cmp channel-map files. " "Each SPEC is FILE[:START_CHAN[:HS_ID]] " - "(defaults: START_CHAN=1, HS_ID=0 → no label prefix)." + "(defaults: START_CHAN=1, HS_ID=0)." ), ) parser.add_argument( @@ -205,10 +306,7 @@ def main(argv: list[str] | None = None) -> int: nargs="+", type=_parse_spec, metavar="SPEC", - help=( - "one or more FILE:START_CHAN:HS_ID specs " - "(START_CHAN default 1, HS_ID default 0 → no prefix)" - ), + help="one or more FILE:START_CHAN:HS_ID specs (START_CHAN default 1, HS_ID default 0)", ) parser.add_argument( "--dump", diff --git a/pycbsdk/src/pycbsdk/session.py b/pycbsdk/src/pycbsdk/session.py index 2c2c39a0..80ad8181 100644 --- a/pycbsdk/src/pycbsdk/session.py +++ b/pycbsdk/src/pycbsdk/session.py @@ -952,16 +952,16 @@ def get_channels_positions( ) -> list[tuple[int, int, int, int]]: """Get positions from all channels matching a type. - Each position is a 4-tuple ``(x, y, z, w)`` of ``int32`` values, - corresponding to the ``cbPKT_CHANINFO.position[4]`` field. + Each position is a 4-tuple of ``int32`` values from the + ``cbPKT_CHANINFO.position[4]`` field. For channels configured from a + CMP the slots are ``(x, y, size, headstage_id)``. Args: channel_type: Channel type filter (e.g., ``ChannelType.FRONTEND``). n_chans: Max channels to query (0 or omit for all). Returns: - List of ``(x, y, z, w)`` tuples (same order as - :meth:`get_matching_channel_ids`). + List of 4-tuples (same order as :meth:`get_matching_channel_ids`). """ _lib = _get_lib() max_chans = _lib.cbsdk_get_max_chans() @@ -1284,22 +1284,24 @@ def configure_channel(self, chan_id: int, *, auto_sync: bool = False, **kwargs): def load_channel_map(self, filepath: str, start_chan: int = 1, hs_id: int = 0): """Load a channel mapping file (.cmp) for one headstage. - CMP files describe one headstage's electrode layout. The file's rows - are sorted by (bank, electrode) and assigned absolute channel IDs - starting at ``start_chan``. Positions are stored locally and overlaid - onto chaninfo; labels are prefixed ``"hs{hs_id}-"`` and pushed to the - device so they persist in chaninfo. + CMP files describe one headstage's electrode layout. Rows are matched + to live channels by device ``(bank, term)``; ``start_chan`` shifts the + file's bank letters by ``start_chan // 32`` banks to select the target + headstage's banks (1 → banks A…, 129 → banks E…). The geometry is + stored locally and overlaid onto chaninfo as + ``position = (x, y, size, headstage_id)``; labels are taken verbatim + and pushed to the device so they persist in chaninfo. Call once per headstage — subsequent calls merge into the overlay, so multiple headstages can coexist on one device. Args: filepath: Path to the .cmp file. - start_chan: 1-based channel to assign the first sorted row. - Typical: 1 for the first headstage, 129 for the second of a + start_chan: 1-based channel selecting the target banks. Typical: + 1 for the first headstage, 129 for the second of a 128-channel headstage, etc. - hs_id: Headstage identifier used to prefix labels. Pass ``0`` - (the default) to leave labels un-prefixed. + hs_id: Headstage identifier stored in the ``headstage`` slot of + ``position`` (0 = none). Does not affect labels. """ _check( _get_lib().cbsdk_session_load_channel_map( diff --git a/pycbsdk/tests/test_cmp.py b/pycbsdk/tests/test_cmp.py index 40057e4a..c684245c 100644 --- a/pycbsdk/tests/test_cmp.py +++ b/pycbsdk/tests/test_cmp.py @@ -10,39 +10,54 @@ from pycbsdk.cmp import CmpEntry, main, parse_cmp -def test_parse_single_file_assigns_chans_from_one(cmp_path: Path): +def test_parse_keys_by_bank_term(cmp_path: Path): + # 96-channel file: banks A, B, C (1..3) each with 32 electrodes. + # start_chan=1 → bank offset 0, so keys are (bank, term) for banks 1..3. entries = parse_cmp(cmp_path) assert len(entries) == 96 - assert set(entries) == set(range(1, 97)) assert all(isinstance(e, CmpEntry) for e in entries.values()) + expected_keys = {(bank, term) for bank in (1, 2, 3) for term in range(1, 33)} + assert set(entries) == expected_keys + for (bank, term), entry in entries.items(): + assert entry.bank == bank + assert entry.term == term -def test_parse_labels_are_prefixed_with_hs_id(cmp_path: Path): +def test_parse_labels_are_verbatim(cmp_path: Path): + # Labels are taken straight from the file; hs_id never prefixes them. entries = parse_cmp(cmp_path, hs_id=3) for entry in entries.values(): - assert entry.label.startswith("hs3-") + assert not entry.label.startswith("hs") + # The file's first electrode (bank A, term 1) is labelled "chan1". + assert entries[(1, 1)].label == "chan1" -def test_parse_sorts_by_bank_then_electrode(cmp_path: Path): - entries = parse_cmp(cmp_path) - # 96-channel file: banks A, B, C each with 32 electrodes. - # chan 1 → (bank_idx=1, elec=1), chan 33 → (bank_idx=2, elec=1), … - assert entries[1].position[2:] == (1, 1) - assert entries[32].position[2:] == (1, 32) - assert entries[33].position[2:] == (2, 1) - assert entries[64].position[2:] == (2, 32) - assert entries[65].position[2:] == (3, 1) - assert entries[96].position[2:] == (3, 32) +def test_hs_id_sets_headstage_not_label(cmp_path: Path): + # Same file with hs_id=0 vs hs_id=3: identical labels, differing headstage. + bare = parse_cmp(cmp_path) + with_hs = parse_cmp(cmp_path, hs_id=3) + for key, entry in with_hs.items(): + assert entry.headstage == 3 + assert bare[key].headstage == 0 + assert entry.label == bare[key].label -def test_parse_start_chan_offsets_chan_ids(cmp_path: Path): +def test_parse_start_chan_offsets_banks(cmp_path: Path): + # start_chan=129 → offset 129 // 32 = 4, so CMP banks A,B,C → device 5,6,7. entries = parse_cmp(cmp_path, start_chan=129, hs_id=2) - assert set(entries) == set(range(129, 129 + 96)) - assert entries[129].position[2:] == (1, 1) # bank A elec 1 - assert all(e.label.startswith("hs2-") for e in entries.values()) - - -def test_parse_unsorted_rows(tmp_path: Path): + assert len(entries) == 96 + assert {bank for bank, _ in entries} == {5, 6, 7} + # chan1 row "0 5 A 1 chan1" → device (bank 5, term 1). Default index grid + # → ×400: x=0, y=2000, size=400. + first = entries[(5, 1)] + assert (first.x, first.y) == (0, 2000) + assert first.size == 400 + assert first.label == "chan1" + assert all(e.headstage == 2 for e in entries.values()) + + +def test_parse_matches_by_bank_term(tmp_path: Path): + # Row order doesn't matter — entries are keyed by (bank, term). f = tmp_path / "unsorted.cmp" f.write_text( "// scratch\n" @@ -53,20 +68,90 @@ def test_parse_unsorted_rows(tmp_path: Path): "0 0 A 2 label_a2\n" ) entries = parse_cmp(f, hs_id=1) - # (A,1), (A,2), (A,3), (B,1) → chans 1..4 - assert entries[1].label == "hs1-label_a1" - assert entries[2].label == "hs1-label_a2" - assert entries[3].label == "hs1-label_a3" - assert entries[4].label == "hs1-label_b1" + assert entries[(1, 1)].label == "label_a1" + assert entries[(1, 2)].label == "label_a2" + assert entries[(1, 3)].label == "label_a3" + assert entries[(2, 1)].label == "label_b1" + assert all(e.headstage == 1 for e in entries.values()) -def test_parse_default_hs_id_omits_prefix(cmp_path: Path): - """Default hs_id=0 leaves labels unmodified.""" - bare = parse_cmp(cmp_path) - prefixed = parse_cmp(cmp_path, hs_id=3) - for chan_id, entry in bare.items(): - assert not entry.label.startswith("hs") - assert prefixed[chan_id].label == f"hs3-{entry.label}" +def test_header_declared_size_takes_face_value(tmp_path: Path): + # A header naming a size column → parsed by name; with size present the + # col/row/size values are taken at face value (no µm scaling). + f = tmp_path / "size.cmp" + f.write_text( + "// scratch\n" + "size test\n" + "//col row bank elec size label\n" + "0 0 A 1 4 elec_a1\n" + "1 0 A 2 7 elec_a2\n" + "2 0 A 3 0 elec_a3\n" + "3 0 A 4 9\n" # size=9, no label + ) + entries = parse_cmp(f, hs_id=5) + assert len(entries) == 4 + + assert entries[(1, 1)].size == 4 + assert entries[(1, 1)].label == "elec_a1" + assert entries[(1, 2)].size == 7 + assert entries[(1, 2)].label == "elec_a2" + assert entries[(1, 3)].size == 0 + assert entries[(1, 3)].label == "elec_a3" + # Size present, no label → empty label. + assert entries[(1, 4)].size == 9 + assert entries[(1, 4)].label == "" + # Face value: x is the raw col (no ×400) because a size column was supplied. + assert (entries[(1, 4)].x, entries[(1, 4)].y) == (3, 0) + + +def test_header_driven_column_order(tmp_path: Path): + # The header determines column order — here columns are reordered and a + # size column is present, so values are taken at face value. + f = tmp_path / "reorder.cmp" + f.write_text( + "// scratch\n" + "reordered columns\n" + "//label bank elec col row size\n" + "foo A 1 7 9 2\n" + ) + entries = parse_cmp(f) + assert len(entries) == 1 + e = entries[(1, 1)] + assert e.label == "foo" + assert (e.x, e.y, e.size) == (7, 9, 2) + + +def test_non_uniform_grid_takes_face_value(tmp_path: Path): + # No size column, but non-uniform spacing (row delta 4) → not a default + # index grid → face value, size 0. + f = tmp_path / "nonuniform.cmp" + f.write_text( + "// scratch\n" + "non-uniform\n" + "//col row bank elec label\n" + "0 0 A 1 e1\n" + "0 4 A 2 e2\n" + ) + entries = parse_cmp(f) + assert entries[(1, 2)].y == 4 # raw row, not scaled + assert entries[(1, 2)].size == 0 + assert entries[(1, 1)].size == 0 + + +def test_default_grid_scales_to_microns(tmp_path: Path): + # No size column + unit-spaced grid → indices scaled ×400, size = 400. + f = tmp_path / "grid.cmp" + f.write_text( + "// scratch\n" + "unit grid\n" + "0 0 A 1 e1\n" + "1 0 A 2 e2\n" + "0 1 A 3 e3\n" + ) + entries = parse_cmp(f) + assert entries[(1, 2)].x == 400 # col 1 → 400 µm + assert entries[(1, 3)].y == 400 # row 1 → 400 µm + assert all(e.size == 400 for e in entries.values()) def test_parse_rejects_missing_file(tmp_path: Path): @@ -106,5 +191,6 @@ def test_main_dump_prints_entries(cmp_path: Path, capsys): stdout = capsys.readouterr().out assert "start_chan=1" in stdout assert "hs_id=1" in stdout - assert "chan 1" in stdout # formatted with width - assert "hs1-" in stdout + assert "bank=1 term= 1" in stdout # formatted (bank, term) + assert "hs=1" in stdout # headstage column + assert "chan1" in stdout # verbatim label, no prefix diff --git a/pycbsdk/tests/test_configuration.py b/pycbsdk/tests/test_configuration.py index 1edbf37c..53210fee 100644 --- a/pycbsdk/tests/test_configuration.py +++ b/pycbsdk/tests/test_configuration.py @@ -884,6 +884,23 @@ def _frontend_view(session): labels = session.get_channels_labels(ChannelType.FRONTEND) return {c: (p, l) for c, p, l in zip(chan_ids, positions, labels)} + @staticmethod + def _frontend_by_bank_term(session): + """Return ``{(bank, term): (chan_id, position, label)}`` for present FE chans. + + CMP entries are keyed by device ``(bank, term)`` (the same join key the + SDK matches on), so device readback is re-keyed the same way to compare. + """ + chan_ids = session.get_matching_channel_ids(ChannelType.FRONTEND) + positions = session.get_channels_positions(ChannelType.FRONTEND) + labels = session.get_channels_labels(ChannelType.FRONTEND) + out = {} + for c, p, l in zip(chan_ids, positions, labels): + bank = session.get_channel_field(c, ChanInfoField.BANK) + term = session.get_channel_field(c, ChanInfoField.TERM) + out[(bank, term)] = (c, p, l) + return out + def test_load_channel_map_smoke(self, nplay_session, cmp_path): """Loading a CMP and syncing succeeds without raising.""" nplay_session.load_channel_map(str(cmp_path)) @@ -902,76 +919,75 @@ def test_positions_match_parsed(self, nplay_session, cmp_path): nplay_session.sync() expected = parse_cmp(str(cmp_path)) - view = self._frontend_view(nplay_session) + view = self._frontend_by_bank_term(nplay_session) intersect = sorted(set(expected) & set(view)) assert intersect, "no FRONTEND chans overlap parsed CMP entries" - for chan_id in intersect: - actual_pos, _ = view[chan_id] - assert actual_pos == expected[chan_id].position, ( - f"chan {chan_id}: expected {expected[chan_id].position}, got {actual_pos}" + for key in intersect: + _, actual_pos, _ = view[key] + e = expected[key] + assert actual_pos == (e.x, e.y, e.size, e.headstage), ( + f"{key}: expected {(e.x, e.y, e.size, e.headstage)}, got {actual_pos}" ) def test_labels_round_trip_through_device(self, nplay_session, cmp_path): - """After load + sync, labels read back through the device match the prefix.""" + """After load + sync, labels read back through the device verbatim.""" from pycbsdk.cmp import parse_cmp nplay_session.load_channel_map(str(cmp_path), hs_id=7) nplay_session.sync() expected = parse_cmp(str(cmp_path), hs_id=7) - view = self._frontend_view(nplay_session) + view = self._frontend_by_bank_term(nplay_session) intersect = sorted(set(expected) & set(view)) assert intersect - for chan_id in intersect: - _, actual_label = view[chan_id] - assert actual_label == expected[chan_id].label, ( - f"chan {chan_id}: expected {expected[chan_id].label!r}, " - f"got {actual_label!r}" + for key in intersect: + _, _, actual_label = view[key] + assert actual_label == expected[key].label, ( + f"{key}: expected {expected[key].label!r}, got {actual_label!r}" ) - # All labels carry the hs7 prefix we requested. - assert actual_label.startswith("hs7-") + # Labels are verbatim — hs_id never prefixes them. + assert not actual_label.startswith("hs") - def test_hs_id_prefix_changes_label(self, nplay_session, cmp_path): - """Loading with a different hs_id rewrites labels on the device.""" + def test_hs_id_sets_headstage(self, nplay_session, cmp_path): + """hs_id lands in the headstage slot of position, not the label.""" from pycbsdk.cmp import parse_cmp - cmp_chans = set(parse_cmp(str(cmp_path))) - nplay_session.load_channel_map(str(cmp_path), hs_id=4) nplay_session.sync() - before = self._frontend_view(nplay_session) - + before = self._frontend_by_bank_term(nplay_session) + covered = sorted(set(parse_cmp(str(cmp_path), hs_id=4)) & set(before)) + assert covered + for key in covered: + _, pos, label = before[key] + assert pos[3] == 4 # headstage slot (position[3]) carries hs_id + assert not label.startswith("hs") + + # Reloading with a different hs_id updates the headstage slot; the + # (verbatim) label is unchanged. nplay_session.load_channel_map(str(cmp_path), hs_id=5) nplay_session.sync() - after = self._frontend_view(nplay_session) - - # Only chans the CMP covers carry the hs prefix; the rest keep their - # device-default labels. Restrict the check to covered chans that are - # also visible on the device. - relabeled = sorted(set(before) & set(after) & cmp_chans) - assert relabeled - for chan_id in relabeled: - assert before[chan_id][1].startswith("hs4-") - assert after[chan_id][1].startswith("hs5-") - # The original (un-prefixed) label is unchanged across the two loads. - assert before[chan_id][1].split("-", 1)[1] == after[chan_id][1].split("-", 1)[1] + after = self._frontend_by_bank_term(nplay_session) + for key in covered: + _, pos, label = after[key] + assert pos[3] == 5 + assert label == before[key][2] def test_start_chan_assignment( self, nplay_session, manufacturer_cmp_path ): """Loading the same file at start_chan=1 vs 129 produces matching layouts. - Parser output for ``(start_chan=1, hs_id=1)`` and ``(start_chan=129, - hs_id=2)`` must agree on (col, row, bank, electrode) at offset chans — - e.g. chan 1 under hs1 has the same position as chan 129 under hs2 — and - whichever of those chans the device actually reports must reflect the - right layout. + The two loads target disjoint banks (start_chan=129 shifts by 4 banks), + so an electrode at ``(bank, term)`` under hs1 corresponds to + ``(bank+4, term)`` under hs2 with identical geometry/label and differing + only in the headstage id. Whichever chans the device reports must + reflect the right layout. """ from pycbsdk.cmp import parse_cmp - # Load both headstages so chans 1.. and 129.. are populated. + # Load both headstages so banks A.. and E.. are populated. nplay_session.load_channel_map( str(manufacturer_cmp_path), start_chan=1, hs_id=1 ) @@ -983,24 +999,26 @@ def test_start_chan_assignment( hs1 = parse_cmp(str(manufacturer_cmp_path), start_chan=1, hs_id=1) hs2 = parse_cmp(str(manufacturer_cmp_path), start_chan=129, hs_id=2) - # Shared invariant: corresponding (chan, chan+128) entries describe the - # same electrode and differ only in the hs prefix. - for sorted_idx in range(min(len(hs1), len(hs2))): - assert hs1[1 + sorted_idx].position == hs2[129 + sorted_idx].position - assert hs1[1 + sorted_idx].label == hs2[129 + sorted_idx].label.replace( - "hs2-", "hs1-", 1 - ) + # Shared invariant: (bank, term) and (bank+4, term) describe the same + # electrode and differ only in the headstage id. + for (bank, term), e1 in hs1.items(): + e2 = hs2[(bank + 4, term)] + assert (e1.x, e1.y, e1.size) == (e2.x, e2.y, e2.size) + assert e1.label == e2.label + assert e1.headstage == 1 and e2.headstage == 2 # And on the device, every present FE chan that falls in either range # matches its parsed entry. - view = self._frontend_view(nplay_session) - for chan_id, (pos, label) in view.items(): - if chan_id in hs1: - assert pos == hs1[chan_id].position - assert label == hs1[chan_id].label - elif chan_id in hs2: - assert pos == hs2[chan_id].position - assert label == hs2[chan_id].label + view = self._frontend_by_bank_term(nplay_session) + for key, (_, pos, label) in view.items(): + if key in hs1: + e = hs1[key] + elif key in hs2: + e = hs2[key] + else: + continue + assert pos == (e.x, e.y, e.size, e.headstage) + assert label == e.label def test_overlay_survives_chanrep_refresh(self, nplay_session, cmp_path): """A CHANREP echoed back from the device should not erase our overlay. @@ -1016,11 +1034,12 @@ def test_overlay_survives_chanrep_refresh(self, nplay_session, cmp_path): nplay_session.sync() expected = parse_cmp(str(cmp_path), hs_id=9) - view = self._frontend_view(nplay_session) - for chan_id in sorted(set(expected) & set(view)): - pos, label = view[chan_id] - assert pos == expected[chan_id].position - assert label == expected[chan_id].label + view = self._frontend_by_bank_term(nplay_session) + for key in sorted(set(expected) & set(view)): + _, pos, label = view[key] + e = expected[key] + assert pos == (e.x, e.y, e.size, e.headstage) + assert label == e.label def test_clear_channel_map_resets_labels(self, nplay_session, cmp_path): """clear_channel_map() reverts labels to the device default ("chanN").""" @@ -1030,19 +1049,21 @@ def test_clear_channel_map_resets_labels(self, nplay_session, cmp_path): nplay_session.sync() expected = parse_cmp(str(cmp_path), hs_id=11) - view_before = self._frontend_view(nplay_session) + view_before = self._frontend_by_bank_term(nplay_session) loaded = sorted(set(expected) & set(view_before)) assert loaded - for chan_id in loaded: - _, label = view_before[chan_id] - assert label.startswith("hs11-") + # Labels are verbatim from the CMP (no hs prefix). + for key in loaded: + _, _, label = view_before[key] + assert label == expected[key].label + assert not label.startswith("hs") nplay_session.clear_channel_map() nplay_session.sync() - view_after = self._frontend_view(nplay_session) - for chan_id in loaded: - _, label = view_after[chan_id] + view_after = self._frontend_by_bank_term(nplay_session) + for key in loaded: + chan_id, _, label = view_after[key] assert label == f"chan{chan_id}", ( f"chan {chan_id}: expected default label, got {label!r}" ) diff --git a/src/cbsdk/src/cmp_parser.cpp b/src/cbsdk/src/cmp_parser.cpp index e0287a00..8b580822 100644 --- a/src/cbsdk/src/cmp_parser.cpp +++ b/src/cbsdk/src/cmp_parser.cpp @@ -7,22 +7,117 @@ #include #include -#include +#include #include +#include +#include #include namespace cbsdk { namespace { +/// Blackrock Utah arrays have 400 µm inter-electrode spacing. Default index +/// grids (unit-spaced col/row indices, no explicit size) are scaled by this to +/// convert electrode indices to micrometers. +constexpr int32_t kElectrodePitchUm = 400; + +/// A column kind in a CMP data row. +enum class Column { Col, Row, Bank, Elec, Size, Label, Unknown }; + +/// Classify a header token (case-insensitive, punctuation-stripped). +Column classifyColumn(std::string tok) { + for (auto& c : tok) c = static_cast(std::tolower(static_cast(c))); + while (!tok.empty() && !std::isalnum(static_cast(tok.back()))) tok.pop_back(); + while (!tok.empty() && !std::isalnum(static_cast(tok.front()))) tok.erase(tok.begin()); + if (tok == "col" || tok == "column" || tok == "c" || tok == "x") return Column::Col; + if (tok == "row" || tok == "r" || tok == "y") return Column::Row; + if (tok == "bank" || tok == "b") return Column::Bank; + if (tok == "elec" || tok == "electrode" || tok == "e" || tok == "term" || tok == "terminal") + return Column::Elec; + if (tok == "size" || tok == "s" || tok == "sz") return Column::Size; + if (tok == "label" || tok == "l" || tok == "name") return Column::Label; + return Column::Unknown; +} + +/// Resolve the column order from a candidate header line. Returns true and +/// fills @c columns / @c has_size if the line names all of col/row/bank/elec. +bool columnsFromHeader(const std::string& header, std::vector& columns, bool& has_size) { + std::istringstream hs(header); + std::vector cols; + std::string tok; + while (hs >> tok) cols.push_back(classifyColumn(tok)); + auto has = [&](Column c) { return std::find(cols.begin(), cols.end(), c) != cols.end(); }; + if (has(Column::Col) && has(Column::Row) && has(Column::Bank) && has(Column::Elec)) { + has_size = has(Column::Size); // before the move — has() reads cols + columns = std::move(cols); + return true; + } + return false; +} + struct RawEntry { int32_t col; int32_t row; int32_t bank_idx; ///< 1-based (A=1, B=2, …) int32_t electrode; ///< 1-based within bank + int32_t size; ///< from a size column, else 0 std::string label; }; +/// Parse one data row per the resolved column order. Returns false (skip row) +/// if any required column (col/row/bank/elec) is missing or unparseable. +bool parseRow(const std::string& line, const std::vector& columns, RawEntry& out) { + std::istringstream iss(line); + std::vector toks; + std::string t; + while (iss >> t) toks.push_back(std::move(t)); + + out = RawEntry{0, 0, 0, 0, 0, ""}; + bool got_col = false, got_row = false, got_bank = false, got_elec = false; + for (size_t i = 0; i < columns.size(); ++i) { + if (i >= toks.size()) break; // optional trailing columns simply absent + const std::string& tk = toks[i]; + try { + switch (columns[i]) { + case Column::Col: out.col = std::stoi(tk); got_col = true; break; + case Column::Row: out.row = std::stoi(tk); got_row = true; break; + case Column::Elec: out.electrode = std::stoi(tk); got_elec = true; break; + case Column::Size: out.size = std::stoi(tk); break; + case Column::Bank: + if (tk.empty() || !std::isalpha(static_cast(tk[0]))) return false; + out.bank_idx = std::toupper(static_cast(tk[0])) - 'A' + 1; + got_bank = true; + break; + case Column::Label: out.label = tk; break; + case Column::Unknown: break; // consume and ignore + } + } catch (...) { + return false; // non-numeric in a numeric column + } + } + return got_col && got_row && got_bank && got_elec; +} + +/// True if every non-zero delta among the distinct col and row values is +/// exactly 1 — i.e. a contiguous, unit-spaced index grid (the manufacturer +/// default), as opposed to values already in physical units. +bool isUnitSpacedGrid(const std::vector& raw) { + std::set cols, rows; + for (const auto& r : raw) { cols.insert(r.col); rows.insert(r.row); } + std::set deltas; + for (const auto* s : {&cols, &rows}) { + int32_t prev = 0; + bool first = true; + for (int32_t v : *s) { + if (!first) deltas.insert(v - prev); + prev = v; + first = false; + } + } + return deltas.size() == 1 && *deltas.begin() == 1; +} + } // namespace cbutil::Result parseCmpFile( @@ -35,7 +130,11 @@ cbutil::Result parseCmpFile( } std::vector raw; - bool found_description = false; + bool description_seen = false; + std::string header_candidate; // most recent comment line after the description + std::vector columns; + bool columns_resolved = false; + bool has_size = false; std::string line; while (std::getline(file, line)) { @@ -44,54 +143,61 @@ cbutil::Result parseCmpFile( line.pop_back(); } if (line.empty()) continue; - if (line.size() >= 2 && line[0] == '/' && line[1] == '/') continue; - // First non-comment line is the description — skip it - if (!found_description) { - found_description = true; + if (line.size() >= 2 && line[0] == '/' && line[1] == '/') { + // A comment after the description may be the column header. + if (description_seen) header_candidate = line.substr(2); continue; } - // Data line: col row bank electrode [label] - std::istringstream iss(line); - int32_t col = 0, row = 0, electrode = 0; - std::string bank_str; - if (!(iss >> col >> row >> bank_str >> electrode)) continue; - if (bank_str.empty() || !std::isalpha(static_cast(bank_str[0]))) continue; - - int32_t bank_idx = std::toupper(static_cast(bank_str[0])) - 'A' + 1; + // First non-comment line is the description — skip it. + if (!description_seen) { + description_seen = true; + header_candidate.clear(); + continue; + } - std::string label; - iss >> label; // optional + // First data row: lock in the column order from the header if it names + // the required columns, else fall back to the legacy positional order. + if (!columns_resolved) { + if (header_candidate.empty() || !columnsFromHeader(header_candidate, columns, has_size)) { + columns = {Column::Col, Column::Row, Column::Bank, Column::Elec, Column::Label}; + has_size = false; + } + columns_resolved = true; + } - raw.push_back({col, row, bank_idx, electrode, std::move(label)}); + RawEntry entry; + if (parseRow(line, columns, entry)) raw.push_back(std::move(entry)); } if (raw.empty()) { return cbutil::Result::error("No valid entries found in CMP file: " + filepath); } - // Rows are not guaranteed to be in channel order. Sort so the Nth - // sorted entry maps to (start_chan + N). - std::sort(raw.begin(), raw.end(), [](const RawEntry& a, const RawEntry& b) { - if (a.bank_idx != b.bank_idx) return a.bank_idx < b.bank_idx; - return a.electrode < b.electrode; - }); + // When no size column is supplied and the col/row values form a unit-spaced + // index grid, treat them as electrode indices: size is 1 and all of + // x/y/size scale by the 400 µm electrode pitch. Otherwise take col/row (and + // any supplied size) at face value. + const bool scale = !has_size && isUnitSpacedGrid(raw); + const int32_t factor = scale ? kElectrodePitchUm : 1; - // hs_id == 0 means "single headstage, no prefix needed". - std::string prefix = (hs_id == 0) - ? std::string{} - : "hs" + std::to_string(hs_id) + "-"; + // start_chan selects the target banks: shift every CMP bank by this many + // banks (32 terminals each). start_chan 1 → +0 (banks A…), 129 → +4 (E…). + const int32_t bank_offset = static_cast(start_chan / 32); CmpEntries entries; entries.reserve(raw.size()); - for (size_t i = 0; i < raw.size(); ++i) { - const auto& r = raw[i]; - uint32_t chan_id = start_chan + static_cast(i); + for (auto& r : raw) { CmpEntry entry; - entry.position = {r.col, r.row, r.bank_idx, r.electrode}; - entry.label = prefix + r.label; - entries.emplace(chan_id, std::move(entry)); + entry.x = r.col * factor; + entry.y = r.row * factor; + entry.size = scale ? kElectrodePitchUm : (has_size ? r.size : 0); + entry.headstage = static_cast(hs_id); + entry.bank = r.bank_idx + bank_offset; + entry.term = r.electrode; + entry.label = std::move(r.label); + entries.emplace(cmpKey(entry.bank, entry.term), std::move(entry)); } return cbutil::Result::ok(std::move(entries)); diff --git a/src/cbsdk/src/cmp_parser.h b/src/cbsdk/src/cmp_parser.h index 68c297e9..1b772eb0 100644 --- a/src/cbsdk/src/cmp_parser.h +++ b/src/cbsdk/src/cmp_parser.h @@ -5,24 +5,41 @@ /// CMP files define electrode positions for a single headstage. Format: /// - Lines starting with // are comments /// - First non-comment line is a description string -/// - Subsequent lines: col row bank electrode [label] -/// - col: 0-based column (left to right) -/// - row: 0-based row (bottom to top) +/// - An optional column-header comment immediately before the data names the +/// columns, e.g. "//col row bank elec label" or "//col row bank elec size +/// label". When present it is the ground truth for column order; column +/// names are matched case-insensitively (col/c, row/r, bank/b, elec/e, +/// size/s, label/l), so the order is not assumed. With no header the +/// legacy positional order "col row bank electrode [label]" is used. +/// - Data rows hold: +/// - col: column index or coordinate (left to right) +/// - row: row index or coordinate (bottom to top) /// - bank: letter A-H (32 electrodes per bank within the headstage) /// - electrode: 1-based electrode within bank (1-32) +/// - size: electrode size, same units as col/row (optional) /// - label: free-form label (optional) /// -/// Rows in a CMP file are not guaranteed to be in channel order. The parser -/// sorts by (bank_letter, electrode) and assigns each sorted entry a 1-based -/// absolute channel ID starting at @c start_chan. This is how a CMP gets -/// applied to a contiguous range of channels — callers control the starting -/// channel so multiple CMPs (one per headstage) can be loaded on one device. +/// Units: when no size column is supplied and the col/row values form a +/// unit-spaced index grid (every non-zero delta among the distinct col/row +/// values is exactly 1 — the manufacturer default), they are interpreted as +/// electrode indices: size becomes 1 and x/y/size are scaled by the 400 µm +/// Utah-array electrode pitch. Otherwise (a size column is present, or the +/// spacing is non-uniform) col/row/size are taken at face value. /// -/// Labels are reused across CMP files (e.g. "elec1-1"..."elec1-128" in every -/// file), so the parser prefixes each label with "hs{hs_id}-" to keep them -/// unique on the device. Pass @c hs_id == 0 to skip prefixing entirely -/// (sensible for single-headstage rigs where the original labels are already -/// unique). +/// Each parsed row carries the electrode geometry plus the device (bank, term) +/// it belongs to. Entries are matched to live channels by (bank, term) — read +/// from the device's own chaninfo — rather than by an ordinal channel ID, so a +/// CMP that is missing channels still lands on the right ones. @c start_chan +/// selects which physical banks the CMP targets: the CMP's bank letters are +/// offset by @c start_chan/32 banks. e.g. a @c start_chan of 129 (= 4 banks) +/// maps CMP bank A onto device bank E, bank C onto bank G, and so on. This is +/// how multiple CMPs (one per headstage) land on disjoint banks of one device. +/// +/// @c hs_id identifies the headstage and is stored in each entry's +/// @c headstage field (→ position[3]). Labels are taken verbatim from the CMP +/// file — even though labels are reused across CMP files (e.g. "elec1-1" in +/// every headstage's file), the stored headstage id disambiguates them, so no +/// "hs{hs_id}-" label prefix is applied. /// /////////////////////////////////////////////////////////////////////////////////////////////////// @@ -30,36 +47,55 @@ #define CBSDK_CMP_PARSER_H #include -#include #include #include #include namespace cbsdk { -/// One parsed CMP entry, ready to apply to an absolute channel. +/// One parsed CMP entry. The geometry fields (@c x, @c y, @c size, +/// @c headstage) populate cbPKT_CHANINFO.position[0..3]; the device +/// (@c bank, @c term) are the keys used to find which channel that is. struct CmpEntry { - std::array position; ///< {col, row, bank_letter_idx, electrode} - std::string label; ///< prefixed label (e.g. "hs1-chan12") + int32_t x = 0; ///< electrode x (µm for index grids, else face value) → position[0] + int32_t y = 0; ///< electrode y (µm for index grids, else face value) → position[1] + int32_t size = 0; ///< electrode size, same units as x/y (0 = unspecified) → position[2] + int32_t headstage = 0; ///< 1-based headstage id (== hs_id; 0 = none) → position[3] + int32_t bank = 0; ///< 1-based device bank (CMP bank + start_chan/32); match key + int32_t term = 0; ///< 1-based terminal within bank (CMP electrode); match key + std::string label; ///< verbatim label (e.g. "chan12") }; -/// Map from 1-based absolute channel ID to parsed entry. +/// Encode a (bank, term) pair into a single CmpEntries lookup key. +/// @c bank is the 1-based device bank, @c term the 1-based terminal within +/// it. Collision-free for the valid hardware ranges (bank ≤ 8, term ≤ 32). +inline uint32_t cmpKey(const int32_t bank, const int32_t term) { + return (static_cast(bank) << 8) | (static_cast(term) & 0xFFu); +} + +/// Map from cmpKey(bank, term) to the parsed entry for that device terminal. using CmpEntries = std::unordered_map; -/// Parse a CMP file and assign each row an absolute channel ID. +/// Parse a CMP file into entries keyed by device (bank, term). +/// +/// Columns come from the header comment immediately before the data when +/// present (matched by name, any order), else the legacy "col row bank +/// electrode [label]" order. col/row/size are scaled from electrode indices to +/// µm for a unit-spaced index grid with no size column, and taken at face value +/// otherwise (see the file header for details). /// -/// Rows are sorted by (bank_letter, electrode) and the Nth sorted row is -/// assigned @c chan_id = @c start_chan + N. Labels get an "hs{hs_id}-" -/// prefix. +/// Each row's CMP bank letter is offset by @c start_chan/32 banks to produce +/// the target device bank; the electrode becomes the terminal. Callers join +/// these against live chaninfo by (bank, term). /// /// @param filepath Path to the .cmp file. -/// @param start_chan 1-based channel to assign the first sorted entry. -/// Typical values: 1 (single headstage), 129 (second +/// @param start_chan 1-based channel selecting the target banks: the CMP is +/// shifted by @c start_chan/32 banks. Typical values: 1 +/// (banks A…, single headstage), 129 (banks E…, second /// headstage of 128 channels), etc. -/// @param hs_id Headstage identifier used to prefix labels. The final -/// label is "hs{hs_id}-{original_label}". Pass 0 (the -/// default) to leave labels un-prefixed. -/// @return Map of chan_id → CmpEntry on success, or error message. +/// @param hs_id Headstage identifier stored in each entry's @c headstage +/// field (→ position[3]; 0 = none). Does not affect labels. +/// @return Map of cmpKey(bank, term) → CmpEntry on success, or error message. cbutil::Result parseCmpFile( const std::string& filepath, uint32_t start_chan = 1, diff --git a/src/cbsdk/src/sdk_session.cpp b/src/cbsdk/src/sdk_session.cpp index 2e48e3ff..08423471 100644 --- a/src/cbsdk/src/sdk_session.cpp +++ b/src/cbsdk/src/sdk_session.cpp @@ -281,8 +281,8 @@ struct SdkSession::Impl { std::array channel_type_cache; bool channel_cache_valid = false; - // CMP (channel mapping) overlay. Keyed by 1-based absolute chan_id → - // {position[4], prefixed label}. Loaded rarely, read on every CHANREP. + // CMP (channel mapping) overlay. Keyed by cmpKey(bank, term) → + // {x, y, size, headstage, label}. Loaded rarely, read on every CHANREP. CmpEntries cmp_entries; std::mutex cmp_mutex; @@ -369,16 +369,15 @@ struct SdkSession::Impl { // (e.g., during the handshake phase in start()). std::atomic shutting_down{false}; - /// Apply CMP overlay (position + label) to all existing chaninfo entries. - /// Called after loading a CMP file. Writes the updated chaninfo to shmem. - /// Label push to the device is done separately by SdkSession::loadChannelMap. + /// Apply CMP overlay (position + label) to every channel whose device + /// (bank, term) matches a loaded CMP entry. Called after loading a CMP + /// file. Writes the updated chaninfo to shmem. Label push to the device + /// is done separately by SdkSession::loadChannelMap. void applyCmpToAllChannels() { std::lock_guard lock(cmp_mutex); if (cmp_entries.empty()) return; - for (const auto& [chan_id, entry] : cmp_entries) { - if (chan_id < 1 || chan_id > cbMAXCHANS) continue; - + for (uint32_t chan_id = 1; chan_id <= cbMAXCHANS; ++chan_id) { // Snapshot chaninfo to avoid racing with the receive thread cbPKT_CHANINFO ci{}; bool valid = false; @@ -391,8 +390,15 @@ struct SdkSession::Impl { } if (!valid) continue; - std::memcpy(ci.position, entry.position.data(), sizeof(int32_t) * 4); - std::strncpy(ci.label, entry.label.c_str(), sizeof(ci.label) - 1); + // Match the CMP row by the channel's own (bank, term). + auto it = cmp_entries.find(cmpKey(ci.bank, ci.term)); + if (it == cmp_entries.end()) continue; + + ci.position[0] = it->second.x; + ci.position[1] = it->second.y; + ci.position[2] = it->second.size; + ci.position[3] = it->second.headstage; + std::strncpy(ci.label, it->second.label.c_str(), sizeof(ci.label) - 1); ci.label[sizeof(ci.label) - 1] = '\0'; if (shmem_session) { @@ -935,11 +941,13 @@ Result SdkSession::start() { { std::lock_guard lock(impl->cmp_mutex); if (!impl->cmp_entries.empty()) { - auto it = impl->cmp_entries.find(chaninfo_copy.chan); + auto it = impl->cmp_entries.find( + cmpKey(chaninfo_copy.bank, chaninfo_copy.term)); if (it != impl->cmp_entries.end()) { - std::memcpy(chaninfo_copy.position, - it->second.position.data(), - sizeof(int32_t) * 4); + chaninfo_copy.position[0] = it->second.x; + chaninfo_copy.position[1] = it->second.y; + chaninfo_copy.position[2] = it->second.size; + chaninfo_copy.position[3] = it->second.headstage; std::strncpy(chaninfo_copy.label, it->second.label.c_str(), sizeof(chaninfo_copy.label) - 1); @@ -2104,14 +2112,11 @@ Result SdkSession::loadChannelMap( return Result::error(parse_result.error()); } - // Snapshot labels before moving entries into the overlay map — we - // need a stable list to push to the device outside the cmp_mutex. - std::vector> labels_to_push; + // Merge parsed entries (keyed by device bank/term) into the overlay map. { std::lock_guard lock(m_impl->cmp_mutex); - for (auto& [chan_id, entry] : parse_result.value()) { - labels_to_push.emplace_back(chan_id, entry.label); - m_impl->cmp_entries[chan_id] = std::move(entry); + for (auto& [key, entry] : parse_result.value()) { + m_impl->cmp_entries[key] = std::move(entry); } } @@ -2120,8 +2125,22 @@ Result SdkSession::loadChannelMap( // Push labels to the device so they persist in chaninfo and are echoed // back in future CHANREP packets. Positions aren't persisted by the - // device, so no analogous push for position. + // device, so no analogous push for position. We discover which channels + // matched a CMP row by joining live chaninfo on (bank, term) — the same + // way applyCmpToAllChannels does — then snapshot (chan_id, label) so the + // network sends happen outside cmp_mutex. if (m_impl->device_session || m_impl->shmem_session) { + std::vector> labels_to_push; + { + std::lock_guard lock(m_impl->cmp_mutex); + for (uint32_t chan_id = 1; chan_id <= cbMAXCHANS; ++chan_id) { + const cbPKT_CHANINFO* info = getChanInfo(chan_id); + if (!info || info->chan == 0) continue; + auto it = m_impl->cmp_entries.find(cmpKey(info->bank, info->term)); + if (it == m_impl->cmp_entries.end()) continue; + labels_to_push.emplace_back(chan_id, it->second.label); + } + } for (const auto& [chan_id, label] : labels_to_push) { const cbPKT_CHANINFO* info = getChanInfo(chan_id); if (!info) continue; @@ -2138,15 +2157,20 @@ Result SdkSession::loadChannelMap( } Result SdkSession::clearChannelMap() { - // Snapshot the previously-mapped channel ids and drop the overlay so - // future CHANREPs land in chaninfo as the device sends them. Any further - // applyCmpToAllChannels() call is now a no-op. + // Find which channels currently match a loaded CMP entry (by bank/term), + // then drop the overlay so future CHANREPs land in chaninfo as the device + // sends them. Any further applyCmpToAllChannels() call is now a no-op. std::vector mapped_chans; { std::lock_guard lock(m_impl->cmp_mutex); - mapped_chans.reserve(m_impl->cmp_entries.size()); - for (const auto& [chan_id, _] : m_impl->cmp_entries) { - mapped_chans.push_back(chan_id); + if (!m_impl->cmp_entries.empty()) { + for (uint32_t chan_id = 1; chan_id <= cbMAXCHANS; ++chan_id) { + const cbPKT_CHANINFO* info = getChanInfo(chan_id); + if (!info || info->chan == 0) continue; + if (m_impl->cmp_entries.count(cmpKey(info->bank, info->term))) { + mapped_chans.push_back(chan_id); + } + } } m_impl->cmp_entries.clear(); } diff --git a/tests/unit/test_cmp_parser.cpp b/tests/unit/test_cmp_parser.cpp index c38cfc46..5ef4296c 100644 --- a/tests/unit/test_cmp_parser.cpp +++ b/tests/unit/test_cmp_parser.cpp @@ -27,10 +27,27 @@ TEST(CmpParser, Parse8Channel) { const auto& entries = result.value(); EXPECT_EQ(entries.size(), 8u); - // Defaults: start_chan=1, hs_id=0 → chans 1..8, no label prefix. - std::set chans; - for (const auto& [chan_id, _] : entries) chans.insert(chan_id); - EXPECT_EQ(chans, std::set({1, 2, 3, 4, 5, 6, 7, 8})); + // 8ch file is bank A, electrodes 1..8. Default start_chan=1 → bank offset 0, + // so entries are keyed by (bank 1, term 1..8). + for (int32_t term = 1; term <= 8; ++term) { + auto it = entries.find(cbsdk::cmpKey(1, term)); + ASSERT_NE(it, entries.end()) << "missing (bank 1, term " << term << ")"; + EXPECT_EQ(it->second.bank, 1); + EXPECT_EQ(it->second.term, term); + } + + // No size column + unit-spaced index grid → scaled by the 400 µm pitch. + // Row "0 1 A 1" → x=0, y=400; row "0 0 A 5" → x=0, y=0. + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 1)).x, 0); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 1)).y, 400); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 5)).x, 0); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 5)).y, 0); + + // Default index grid → size is 1 electrode-pitch (400 µm); hs_id=0 → headstage 0. + for (const auto& [_, entry] : entries) { + EXPECT_EQ(entry.size, 400); + EXPECT_EQ(entry.headstage, 0); + } // hs_id=0 leaves labels un-prefixed. for (const auto& [_, entry] : entries) { @@ -38,17 +55,116 @@ TEST(CmpParser, Parse8Channel) { } } -TEST(CmpParser, HsIdZeroLeavesLabelsUnprefixed) { - // Same file, default hs_id=0 vs explicit hs_id=3, comparing stripped labels. +TEST(CmpParser, HeaderDeclaredSizeColumnTakesFaceValue) { + // A header naming a size column → columns parsed by name; with size present + // the col/row/size values are taken at face value (no µm scaling). + auto path = (std::filesystem::temp_directory_path() / "cmp_size_tmp.cmp").string(); + { + std::ofstream out(path); + out << "// scratch file\n" + << "size column test map\n" + << "//col row bank elec size label\n" + << "0 0 A 1 4 elec_a1\n" // size=4, label=elec_a1 + << "1 0 A 2 7 elec_a2\n" // size=7, label=elec_a2 + << "2 0 A 3 0 elec_a3\n" // size=0, label=elec_a3 + << "3 0 A 4 9\n"; // size=9, no label + } + + auto result = cbsdk::parseCmpFile(path, /*start_chan=*/1, /*hs_id=*/5); + ASSERT_TRUE(result.isOk()) << result.error(); + + const auto& entries = result.value(); + ASSERT_EQ(entries.size(), 4u); + + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 1)).size, 4); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 1)).label, "elec_a1"); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 2)).size, 7); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 2)).label, "elec_a2"); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 3)).size, 0); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 3)).label, "elec_a3"); + + // Size present, no label → empty label. + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 4)).size, 9); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 4)).label, ""); + + // Face value: x is the raw col (no ×400) because a size column was supplied. + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 4)).x, 3); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 4)).y, 0); + + // hs_id seeds the headstage field (→ position[3]). + for (const auto& [_, entry] : entries) { + EXPECT_EQ(entry.headstage, 5); + } + + std::filesystem::remove(path); +} + +TEST(CmpParser, HeaderDrivenColumnOrder) { + // The header determines column order — here columns are reordered and a + // size column is present, so values are taken at face value. + auto path = (std::filesystem::temp_directory_path() / "cmp_reorder_tmp.cmp").string(); + { + std::ofstream out(path); + out << "// scratch file\n" + << "reordered columns map\n" + << "//label bank elec col row size\n" + << "foo A 1 7 9 2\n"; + } + + auto result = cbsdk::parseCmpFile(path); + ASSERT_TRUE(result.isOk()) << result.error(); + + const auto& entries = result.value(); + ASSERT_EQ(entries.size(), 1u); + const auto& e = entries.at(cbsdk::cmpKey(1, 1)); + EXPECT_EQ(e.label, "foo"); + EXPECT_EQ(e.bank, 1); + EXPECT_EQ(e.term, 1); + EXPECT_EQ(e.x, 7); + EXPECT_EQ(e.y, 9); + EXPECT_EQ(e.size, 2); +} + +TEST(CmpParser, NonUniformGridTakesFaceValue) { + // No size column, but the row spacing is non-uniform (delta 4, not 1), so + // it is not a default index grid → values taken at face value, size 0. + auto path = (std::filesystem::temp_directory_path() / "cmp_nonuniform_tmp.cmp").string(); + { + std::ofstream out(path); + out << "// scratch file\n" + << "non-uniform grid map\n" + << "//col row bank elec label\n" + << "0 0 A 1 e1\n" + << "0 4 A 2 e2\n"; // row delta 4 → not unit-spaced + } + + auto result = cbsdk::parseCmpFile(path); + ASSERT_TRUE(result.isOk()) << result.error(); + + const auto& entries = result.value(); + ASSERT_EQ(entries.size(), 2u); + // Face value: y is the raw row (4), not scaled; size stays unspecified (0). + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 2)).y, 4); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 2)).size, 0); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 1)).size, 0); +} + +TEST(CmpParser, HsIdSetsHeadstageNotLabel) { + // hs_id is stored in the headstage field, never mixed into the label. + // The same file parsed with hs_id=0 vs hs_id=3 yields identical labels, + // differing only in the headstage value. auto bare = cbsdk::parseCmpFile(testFile("8ChannelDefaultMapping.cmp")); auto with_hs = cbsdk::parseCmpFile(testFile("8ChannelDefaultMapping.cmp"), 1, 3); ASSERT_TRUE(bare.isOk()); ASSERT_TRUE(with_hs.isOk()); - for (const auto& [chan_id, prefixed] : with_hs.value()) { - const auto it = bare.value().find(chan_id); + for (const auto& [key, entry] : with_hs.value()) { + const auto it = bare.value().find(key); ASSERT_NE(it, bare.value().end()); - EXPECT_EQ(prefixed.label, "hs3-" + it->second.label); + EXPECT_EQ(entry.label, it->second.label); // label unchanged by hs_id + EXPECT_EQ(entry.headstage, 3); + EXPECT_EQ(it->second.headstage, 0); + EXPECT_NE(entry.label.substr(0, 2), "hs"); // no "hs3-" prefix } } @@ -59,23 +175,22 @@ TEST(CmpParser, Parse128Channel) { const auto& entries = result.value(); EXPECT_EQ(entries.size(), 128u); - // 128 contiguous channel IDs starting at 1. - for (uint32_t ch = 1; ch <= 128; ++ch) { - ASSERT_TRUE(entries.count(ch)) << "missing chan " << ch; + // 128ch = banks A..D (1..4), each electrodes 1..32. start_chan=1 → offset 0. + for (int32_t bank = 1; bank <= 4; ++bank) { + for (int32_t term = 1; term <= 32; ++term) { + auto it = entries.find(cbsdk::cmpKey(bank, term)); + ASSERT_NE(it, entries.end()) + << "missing (bank " << bank << ", term " << term << ")"; + EXPECT_EQ(it->second.bank, bank); + EXPECT_EQ(it->second.term, term); + } } - - // Bank layout after sort: chan 1 → (bank 1, elec 1), chan 33 → (bank 2, elec 1), - // chan 65 → (bank 3, elec 1), chan 128 → (bank 4, elec 32). - EXPECT_EQ(entries.at(1).position[2], 1); - EXPECT_EQ(entries.at(1).position[3], 1); - EXPECT_EQ(entries.at(33).position[2], 2); - EXPECT_EQ(entries.at(33).position[3], 1); - EXPECT_EQ(entries.at(128).position[2], 4); - EXPECT_EQ(entries.at(128).position[3], 32); } -TEST(CmpParser, StartChanOffsetsChanIds) { - // Second headstage: same 96-channel CMP mapped to chans 129..224. +TEST(CmpParser, StartChanOffsetsBanks) { + // Second headstage: same 96-channel CMP (banks A,B,C) mapped onto the + // device's second set of banks. start_chan=129 → offset 129/32 = 4, so + // CMP bank A→E (5), B→F (6), C→G (7). auto result = cbsdk::parseCmpFile( testFile("96ChannelDefaultMapping.cmp"), /*start_chan=*/129, /*hs_id=*/2); ASSERT_TRUE(result.isOk()) << result.error(); @@ -83,24 +198,39 @@ TEST(CmpParser, StartChanOffsetsChanIds) { const auto& entries = result.value(); EXPECT_EQ(entries.size(), 96u); - std::set chans; - for (const auto& [chan_id, _] : entries) chans.insert(chan_id); - EXPECT_EQ(*chans.begin(), 129u); - EXPECT_EQ(*chans.rbegin(), 129u + 95u); + // Every entry sits in device banks 5, 6 or 7. + std::set banks; + for (const auto& [_, entry] : entries) banks.insert(entry.bank); + EXPECT_EQ(banks, std::set({5, 6, 7})); + // chan1 row "0 5 A 1" → device (bank 5, term 1). Default index grid → ×400, + // so x=0, y=2000, size=400. + auto first = entries.find(cbsdk::cmpKey(5, 1)); + ASSERT_NE(first, entries.end()); + EXPECT_EQ(first->second.x, 0); + EXPECT_EQ(first->second.y, 2000); + EXPECT_EQ(first->second.size, 400); + + // chan96 row "15 0 C 32" → device (bank 7, term 32). + ASSERT_TRUE(entries.count(cbsdk::cmpKey(7, 32))); + + // Labels are verbatim from the file (no "hs2-" prefix); hs_id lands in + // the headstage field instead. + EXPECT_EQ(entries.at(cbsdk::cmpKey(5, 1)).label, "chan1"); for (const auto& [_, entry] : entries) { - EXPECT_EQ(entry.label.substr(0, 4), "hs2-"); + EXPECT_NE(entry.label.substr(0, 2), "hs"); + EXPECT_EQ(entry.headstage, 2); // hs_id → headstage (position[3]) } } -TEST(CmpParser, SortsOutOfOrderRows) { - // Synthesize a tiny CMP whose rows are deliberately out of (bank, elec) order. +TEST(CmpParser, MatchesRowsByBankAndTerm) { + // Rows given out of (bank, elec) order — order no longer matters since + // entries are keyed by (bank, term), not an ordinal channel id. auto path = (std::filesystem::temp_directory_path() / "cmp_unsorted_tmp.cmp").string(); { std::ofstream out(path); out << "// scratch file\n" << "unsorted test map\n" - // Rows given in reverse electrode order within bank A, then bank B first. << "0 0 B 1 label_b1\n" << "0 0 A 3 label_a3\n" << "0 0 A 1 label_a1\n" @@ -114,11 +244,10 @@ TEST(CmpParser, SortsOutOfOrderRows) { const auto& entries = result.value(); ASSERT_EQ(entries.size(), 4u); - // Sort puts (A,1), (A,2), (A,3), (B,1) at chans 1..4. - EXPECT_EQ(entries.at(1).label, "hs1-label_a1"); - EXPECT_EQ(entries.at(2).label, "hs1-label_a2"); - EXPECT_EQ(entries.at(3).label, "hs1-label_a3"); - EXPECT_EQ(entries.at(4).label, "hs1-label_b1"); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 1)).label, "label_a1"); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 2)).label, "label_a2"); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 3)).label, "label_a3"); + EXPECT_EQ(entries.at(cbsdk::cmpKey(2, 1)).label, "label_b1"); std::filesystem::remove(path); } From 8e97f665bf69588d1e654140379d1987b9e6dadc Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Wed, 3 Jun 2026 17:28:57 -0400 Subject: [PATCH 3/4] CMP: Check for unit-indexing (min-spacing) rather than all grid spacing, because some arrays will have gaps in their grid and therefore have >1 spacing in some dimensions. --- pycbsdk/src/pycbsdk/cmp.py | 28 ++++++++++++++--------- pycbsdk/tests/test_cmp.py | 31 ++++++++++++++++++++----- src/cbsdk/src/cmp_parser.cpp | 21 ++++++++--------- src/cbsdk/src/cmp_parser.h | 12 +++++----- tests/unit/test_cmp_parser.cpp | 41 +++++++++++++++++++++++++++++----- 5 files changed, 96 insertions(+), 37 deletions(-) diff --git a/pycbsdk/src/pycbsdk/cmp.py b/pycbsdk/src/pycbsdk/cmp.py index a011d5db..dd35f917 100644 --- a/pycbsdk/src/pycbsdk/cmp.py +++ b/pycbsdk/src/pycbsdk/cmp.py @@ -9,11 +9,13 @@ order ``col row bank electrode [label]`` is used. Units: when no size column is supplied and the col/row values form a -unit-spaced index grid (every non-zero delta among the distinct col/row values -is exactly 1 — the manufacturer default), they are interpreted as electrode -indices: ``size`` becomes 1 and x/y/size are scaled by the 400 µm Utah-array -electrode pitch. Otherwise (a size column is present, or the spacing is -non-uniform) col/row/size are taken at face value. +unit-indexed grid (the smallest non-zero delta among the distinct col/row +values is 1 — so some electrodes are adjacent, the manufacturer default), they +are interpreted as electrode indices: ``size`` becomes 1 and x/y/size are +scaled by the 400 µm Utah-array electrode pitch. Larger deltas (e.g. the gap +between two arrays in a multi-array map) are allowed and scale through. +Otherwise (a size column is present, or no two electrodes are unit-spaced) +col/row/size are taken at face value. Entries are keyed by device ``(bank, term)`` — the same join key the C++ SDK uses to match rows to live channels — not by an ordinal channel ID. Each @@ -133,13 +135,17 @@ def _parse_row(tokens: list[str], columns: list[str]) -> dict | None: return out -def _is_unit_spaced_grid(raw: list[dict]) -> bool: - """True if every non-zero delta among distinct col and row values is 1.""" +def _is_unit_indexed_grid(raw: list[dict]) -> bool: + """True if the smallest non-zero delta among distinct col/row values is 1. + + A single unit step is enough — larger deltas (e.g. the gap between two + arrays in a multi-array map) are allowed and scale through. + """ deltas: set[int] = set() for key in ("col", "row"): vals = sorted({r[key] for r in raw}) deltas.update(b - a for a, b in zip(vals, vals[1:])) - return deltas == {1} + return 1 in deltas def parse_cmp( @@ -194,9 +200,9 @@ def parse_cmp( if not raw: raise ValueError(f"{filepath}: no valid entries") - # No size column + a unit-spaced index grid → electrode indices: size is 1 - # and x/y/size scale by the 400 µm pitch. Otherwise take values at face value. - scale = not has_size and _is_unit_spaced_grid(raw) + # No size column + a unit-indexed grid → electrode indices: size is 1 and + # x/y/size scale by the 400 µm pitch. Otherwise take values at face value. + scale = not has_size and _is_unit_indexed_grid(raw) factor = _PITCH_UM if scale else 1 bank_offset = start_chan // 32 diff --git a/pycbsdk/tests/test_cmp.py b/pycbsdk/tests/test_cmp.py index c684245c..a605fdef 100644 --- a/pycbsdk/tests/test_cmp.py +++ b/pycbsdk/tests/test_cmp.py @@ -121,13 +121,13 @@ def test_header_driven_column_order(tmp_path: Path): assert (e.x, e.y, e.size) == (7, 9, 2) -def test_non_uniform_grid_takes_face_value(tmp_path: Path): - # No size column, but non-uniform spacing (row delta 4) → not a default - # index grid → face value, size 0. - f = tmp_path / "nonuniform.cmp" +def test_no_unit_step_takes_face_value(tmp_path: Path): + # No size column and no two electrodes unit-spaced (only delta is 4) → not + # a unit-indexed grid → face value, size 0. + f = tmp_path / "nounit.cmp" f.write_text( "// scratch\n" - "non-uniform\n" + "no unit step\n" "//col row bank elec label\n" "0 0 A 1 e1\n" "0 4 A 2 e2\n" @@ -154,6 +154,27 @@ def test_default_grid_scales_to_microns(tmp_path: Path): assert all(e.size == 400 for e in entries.values()) +def test_multi_array_gap_still_scales(tmp_path: Path): + # Unit-indexed grid with an inter-array gap (rows 0,1,2 then 6 → deltas + # {1,4}). The unit step means it's still indices → scales ×400; the gap + # scales through (row 6 → 2400 µm). Mirrors a real two-array .cmp. + f = tmp_path / "gap.cmp" + f.write_text( + "// scratch\n" + "multi-array gap\n" + "//col row bank elec label\n" + "0 0 A 1 e1\n" + "0 1 A 2 e2\n" + "0 2 A 3 e3\n" + "0 6 A 4 e4\n" + ) + entries = parse_cmp(f) + assert entries[(1, 1)].y == 0 + assert entries[(1, 2)].y == 400 + assert entries[(1, 4)].y == 2400 # gap scaled through + assert all(e.size == 400 for e in entries.values()) + + def test_parse_rejects_missing_file(tmp_path: Path): with pytest.raises(FileNotFoundError): parse_cmp(tmp_path / "nope.cmp") diff --git a/src/cbsdk/src/cmp_parser.cpp b/src/cbsdk/src/cmp_parser.cpp index 8b580822..1e7b22e9 100644 --- a/src/cbsdk/src/cmp_parser.cpp +++ b/src/cbsdk/src/cmp_parser.cpp @@ -99,10 +99,11 @@ bool parseRow(const std::string& line, const std::vector& columns, RawEn return got_col && got_row && got_bank && got_elec; } -/// True if every non-zero delta among the distinct col and row values is -/// exactly 1 — i.e. a contiguous, unit-spaced index grid (the manufacturer -/// default), as opposed to values already in physical units. -bool isUnitSpacedGrid(const std::vector& raw) { +/// True if the smallest non-zero delta among the distinct col and row values is +/// 1 — i.e. a unit-indexed electrode grid, as opposed to values already in +/// physical units. Larger deltas are allowed (e.g. the gap between two arrays +/// in a multi-array map), so a single unit step anywhere is enough. +bool isUnitIndexedGrid(const std::vector& raw) { std::set cols, rows; for (const auto& r : raw) { cols.insert(r.col); rows.insert(r.row); } std::set deltas; @@ -115,7 +116,7 @@ bool isUnitSpacedGrid(const std::vector& raw) { first = false; } } - return deltas.size() == 1 && *deltas.begin() == 1; + return deltas.count(1) > 0; } } // namespace @@ -175,11 +176,11 @@ cbutil::Result parseCmpFile( return cbutil::Result::error("No valid entries found in CMP file: " + filepath); } - // When no size column is supplied and the col/row values form a unit-spaced - // index grid, treat them as electrode indices: size is 1 and all of - // x/y/size scale by the 400 µm electrode pitch. Otherwise take col/row (and - // any supplied size) at face value. - const bool scale = !has_size && isUnitSpacedGrid(raw); + // When no size column is supplied and the col/row values form a unit-indexed + // grid (some adjacent electrodes are 1 apart), treat them as electrode + // indices: size is 1 and all of x/y/size scale by the 400 µm electrode + // pitch. Otherwise take col/row (and any supplied size) at face value. + const bool scale = !has_size && isUnitIndexedGrid(raw); const int32_t factor = scale ? kElectrodePitchUm : 1; // start_chan selects the target banks: shift every CMP bank by this many diff --git a/src/cbsdk/src/cmp_parser.h b/src/cbsdk/src/cmp_parser.h index 1b772eb0..78da9ad6 100644 --- a/src/cbsdk/src/cmp_parser.h +++ b/src/cbsdk/src/cmp_parser.h @@ -20,11 +20,13 @@ /// - label: free-form label (optional) /// /// Units: when no size column is supplied and the col/row values form a -/// unit-spaced index grid (every non-zero delta among the distinct col/row -/// values is exactly 1 — the manufacturer default), they are interpreted as -/// electrode indices: size becomes 1 and x/y/size are scaled by the 400 µm -/// Utah-array electrode pitch. Otherwise (a size column is present, or the -/// spacing is non-uniform) col/row/size are taken at face value. +/// unit-indexed grid (the smallest non-zero delta among the distinct col/row +/// values is 1 — so some electrodes are adjacent, the manufacturer default), +/// they are interpreted as electrode indices: size becomes 1 and x/y/size are +/// scaled by the 400 µm Utah-array electrode pitch. Larger deltas (e.g. the +/// gap between two arrays in a multi-array map) are allowed and scale through. +/// Otherwise (a size column is present, or no two electrodes are unit-spaced) +/// col/row/size are taken at face value. /// /// Each parsed row carries the electrode geometry plus the device (bank, term) /// it belongs to. Entries are matched to live channels by (bank, term) — read diff --git a/tests/unit/test_cmp_parser.cpp b/tests/unit/test_cmp_parser.cpp index 5ef4296c..f44756d6 100644 --- a/tests/unit/test_cmp_parser.cpp +++ b/tests/unit/test_cmp_parser.cpp @@ -125,17 +125,17 @@ TEST(CmpParser, HeaderDrivenColumnOrder) { EXPECT_EQ(e.size, 2); } -TEST(CmpParser, NonUniformGridTakesFaceValue) { - // No size column, but the row spacing is non-uniform (delta 4, not 1), so - // it is not a default index grid → values taken at face value, size 0. - auto path = (std::filesystem::temp_directory_path() / "cmp_nonuniform_tmp.cmp").string(); +TEST(CmpParser, NoUnitStepTakesFaceValue) { + // No size column and no two electrodes are unit-spaced (only delta is 4), + // so it is not a unit-indexed grid → values taken at face value, size 0. + auto path = (std::filesystem::temp_directory_path() / "cmp_nounit_tmp.cmp").string(); { std::ofstream out(path); out << "// scratch file\n" - << "non-uniform grid map\n" + << "no unit step map\n" << "//col row bank elec label\n" << "0 0 A 1 e1\n" - << "0 4 A 2 e2\n"; // row delta 4 → not unit-spaced + << "0 4 A 2 e2\n"; // only delta is 4 → no unit step } auto result = cbsdk::parseCmpFile(path); @@ -149,6 +149,35 @@ TEST(CmpParser, NonUniformGridTakesFaceValue) { EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 1)).size, 0); } +TEST(CmpParser, MultiArrayGapStillScales) { + // A unit-indexed grid with an inter-array gap (rows 0,1,2 then 6 → deltas + // {1,4}). The unit step means it's still electrode indices, so it scales by + // 400 µm; the gap scales through (row 6 → 2400 µm). + auto path = (std::filesystem::temp_directory_path() / "cmp_gap_tmp.cmp").string(); + { + std::ofstream out(path); + out << "// scratch file\n" + << "multi-array gap map\n" + << "//col row bank elec label\n" + << "0 0 A 1 e1\n" + << "0 1 A 2 e2\n" + << "0 2 A 3 e3\n" + << "0 6 A 4 e4\n"; // gap of 4 between the two arrays + } + + auto result = cbsdk::parseCmpFile(path); + ASSERT_TRUE(result.isOk()) << result.error(); + + const auto& entries = result.value(); + ASSERT_EQ(entries.size(), 4u); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 1)).y, 0); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 2)).y, 400); + EXPECT_EQ(entries.at(cbsdk::cmpKey(1, 4)).y, 2400); // gap scaled through + for (const auto& [_, entry] : entries) EXPECT_EQ(entry.size, 400); + + std::filesystem::remove(path); +} + TEST(CmpParser, HsIdSetsHeadstageNotLabel) { // hs_id is stored in the headstage field, never mixed into the label. // The same file parsed with hs_id=0 vs hs_id=3 yields identical labels, From afb5738ea22dcdecfaada8c8c750b81bc60d5f80 Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Wed, 3 Jun 2026 19:28:40 -0400 Subject: [PATCH 4/4] pycbsdk - ignore shared objects in case we put one here for quick testing --- pycbsdk/.gitignore | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pycbsdk/.gitignore b/pycbsdk/.gitignore index f3c1db24..7528c0ae 100644 --- a/pycbsdk/.gitignore +++ b/pycbsdk/.gitignore @@ -1 +1,6 @@ src/pycbsdk/_version.py +*.dylib +*.dll +*.bin +*.so +*.a