Nest the object loop inside the phase loop in SchemaRawdata.to_kspace - #211
Merged
headmeister merged 2 commits intoAug 10, 2026
Merged
Conversation
`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
The 2-D branch reshaped the PV360 scan stream as
(readout, receivers, phase, objects, repetitions) in Fortran order, which
reads the scan index as phase + Nphase*object.
Spec 5.2 "Acquisition loop nesting (default)" puts it the other way round:
NS > ACQ_phase_factor > NSLICES > NI or NSLICES > NA
> ACQ_size[1]/ACQ_phase_factor > ACQ_size[1] > ACQ_size[2] > NAE > NR
The object level is INSIDE the phase-encode-group level, so consecutive scans
step the object first. The library's own fid path already has this right --
properties_fid_core.json CART_2D encoding_space is
(EncMatrix[0], EncNReceivers, ACQ_phase_factor, NI, EncMatrix[1]//ACQ_phase_factor, NR)
-- so to_kspace disagreed with both the spec and the rest of the library, and
ACQ_phase_factor was missing entirely, mis-nesting RARE-style echo trains too.
Nothing raised: the array kept its shape and dtype, so every 2-D multi-slice
PV360 job -- all of Bruker's standard FLASH/RARE/MSME/MGE protocols -- came
back with its slice and phase-encode indices interleaved.
Adopt the fid nesting. On the PV360 standard data the phase profile now peaks
at exactly N/2 for every 2-D scan, where before it was flat and off-centre:
T1_FLASH 160/288, peak/median 1.26 -> 144/288, 6.46
T1_RARE 170/256, 1.79 -> 128/256, 4.06
T2_TurboRARE 143/256, 3.44 -> 128/256, 10.63
T2map_MSME 187/192, 2.37 -> 96/192, 10.44
and a zero-filled sum-of-squares reconstruction correlates with the vendor
2dseq far better:
T1_FLASH 0.07-0.36 -> 0.47-0.56
T1_RARE 0.03-0.17 -> 0.62-0.99
T2_TurboRARE 0.06-0.22 -> 0.61-0.77
The 3-D branch is unchanged; with NI = 1 there is no object level to interleave.
Closes isi-nmr#192
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017SNrm3jFhpTPShP8xGkePC
gdevenyi
force-pushed
the
fix/rawdata-kspace-loop-nesting
branch
from
August 8, 2026 20:51
e4d671b to
a158f8c
Compare
Contributor
Author
|
Rebased onto #213 ( 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. |
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
2 participants
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 #192.
The 2-D branch reshaped the PV360 scan stream as
(readout, receivers, phase, objects, repetitions)in Fortran order — i.e. it read the scan index asphase + Nphase*object.Spec 5.2, Acquisition loop nesting (default):
The object level is inside the phase-encode-group level, so consecutive scans step the object first. The library's own
fidpath already has this right (properties_fid_core.json,CART_2D), soto_kspacedisagreed with both the spec and the rest of the library — andACQ_phase_factorwas missing from the layout entirely, mis-nesting RARE-style echo trains too.Nothing raised: the array kept its shape and dtype, so every 2-D multi-slice PV360 job came back with its slice and phase-encode indices interleaved.
Evidence
k-space centre, mean |k| along the phase axis (slice 0 / channel 0):
T1_FLASHT1_RARET2_TurboRARET2map_MSMEZero-filled sum-of-squares reconstruction correlated against the vendor
2dseq, per slice:T1_FLASHT1_RARET2_TurboRAREThe 3-D branch is unchanged — with
NI = 1there is no object level to interleave.Test
test_to_kspace_nests_the_object_loop_inside_the_phase_loopbuilds a synthetic PV360 job whose every scan carries1000*object + group, so a misplaced sample is visible, and asserts each lands atk_space[:, group, object]. Fails on master.Suite: 2141 passed, 12 skipped.