Skip to content

Count a job's receivers on its own channel, and give text arrays their declared shape - #206

Merged
headmeister merged 2 commits into
isi-nmr:masterfrom
gdevenyi:fix/receiver-select-per-chan
Aug 10, 2026
Merged

Count a job's receivers on its own channel, and give text arrays their declared shape#206
headmeister merged 2 commits into
isi-nmr:masterfrom
gdevenyi:fix/receiver-select-per-chan

Conversation

@gdevenyi

@gdevenyi gdevenyi commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #201.

Two coupled gaps meant the ACQP-side receiver count of spec 3.3 was never actually computed.

1. Text arrays were delivered flat

GenericParameter.value reshaped numeric arrays to the declared size but returned every text-dtype array flat, so ACQ_ReceiverSelectPerChan declared ( 2, 7 ) arrived as (14,) — with no rows to index.

Spec 2.3 separates the two shapes by element count: a string array carries a final length indicator for the string length (VisuCoreDataUnits=( 2, 65 ) is two strings, not 130), while an enum array carries none (( 2, 7 ) really is two channels of seven receivers). _text_shape picks whichever the count fits. An unrecognised shape stays flat rather than raising — unlike the numeric case, a text array has always been delivered that way. A corpus sweep over 5477 text arrays finds every one matching one rule or the other, none neither.

2. The receiver count ignored chanNum

if channels == fallback:
    return channels
return fallback

made the returned value always PVM_EncNReceivers — and 1 whenever the method omits it — inverting the authority spec 3.3/14.4 give acqp. ACQ_jobs[n][7] was never read.

Now: c = ACQ_jobs[n].chanNum → count Yes in ACQ_ReceiverSelectPerChan row c-1; fall back to the flat ACQ_ReceiverSelect; PVM_EncNReceivers last.

On the six real 2-channel datasets (T2star_map_MGE_mod_all/_mod_pos), ACQ_ReceiverSelectPerChan now arrives as (2, 7), chanNum is read as 1, and the count is 1 — the spec's answer, by the spec's route. All 101 rawdata.* files in the corpus still size exactly.

Tests

  • test_a_text_array_takes_its_declared_shape — the enum, string and 1-D cases.
  • test_rawdata_receiver_count_follows_the_jobs_channel — a synthetic two-job PV360 experiment where channel 1 has one active receiver and channel 2 has three, and PVM_EncNReceivers disagrees with both. Both fail on master.

Suite: 2142 passed, 12 skipped.

gdevenyi and others added 2 commits August 8, 2026 16:33
`ruff format --check` reported twelve files as unformatted, so any change
touching one of them dragged unrelated reflow into its diff. Run the
formatter once, on its own, so subsequent changes show only what they
actually change.

Formatting only: no behaviour changes, and `ruff check` is clean before and
after. The suite is unchanged at 2140 passed, 12 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017SNrm3jFhpTPShP8xGkePC
…r shape

Two coupled gaps meant the ACQP-side receiver count of spec 3.3 was never
actually computed.

GenericParameter.value returned every text-dtype array flat, so a declared
( 2, 7 ) ACQ_ReceiverSelectPerChan arrived as (14,) and there were no rows
to index. Spec 2.3 distinguishes the two shapes by their element count: a
string array carries a final length indicator for the string length --
VisuCoreDataUnits=( 2, 65 ) is two strings -- while an enum array carries
none, so ( 2, 7 ) really is two channels of seven receivers. Reshape to
whichever the count fits; an unrecognised shape stays flat, since unlike the
numeric case a text array has always been delivered that way.

rawdata_channels then counted Yes across the whole flattened array, never
read the job's chanNum, and discarded even that count unless it already
equalled PVM_EncNReceivers -- so the returned value was always the method
parameter, and 1 whenever the method omitted it. Spec 3.3 and 14.4 make the
acqp side authoritative: c = ACQ_jobs[n].chanNum, then count the Yes entries
of row c-1. Follow that, fall back to the flat ACQ_ReceiverSelect, and keep
PVM_EncNReceivers as a last resort rather than the arbiter.

All 101 rawdata files in the corpus still size exactly.

Closes isi-nmr#201

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017SNrm3jFhpTPShP8xGkePC
@gdevenyi
gdevenyi force-pushed the fix/receiver-select-per-chan branch from e48d79c to 0b3ab50 Compare August 8, 2026 20:50
@gdevenyi

gdevenyi commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto #213 (ruff format), so this branch carries that commit until #213 merges — merge #213 first and this diff reduces to its own change. Verified after the rebase: ruff check and ruff format --check clean, full suite green.

Integration check across all eleven conformance PRs merged together: 2164 passed, 12 skipped; corpus load test 3202 → 3207 of 3478 with zero newly broken datasets.

@headmeister
headmeister merged commit 91e5f1f into isi-nmr:master Aug 10, 2026
7 checks passed
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.

Receiver count ignores ACQ_jobs[n].chanNum, and string arrays are never reshaped to their declared dimensions (spec 2.3, 3.3, 14.4)

2 participants