Skip to content

Bruker 2dseq reader: ParaVision on-disk format conformance - #6761

Open
gdevenyi wants to merge 6 commits into
InsightSoftwareConsortium:mainfrom
gdevenyi:bruker-2dseq-format-conformance
Open

Bruker 2dseq reader: ParaVision on-disk format conformance#6761
gdevenyi wants to merge 6 commits into
InsightSoftwareConsortium:mainfrom
gdevenyi:bruker-2dseq-format-conformance

Conversation

@gdevenyi

Copy link
Copy Markdown
Contributor

Parse the JCAMP-DX forms ParaVision writes (PV5.1 headers, PV360 RLE and enum arrays, strings with commas), fix frame-scaling cardinality, and derive slice count and direction from frame groups. Adds a synthetic PV360 GTest, no new external test data.

Defects fixed, per commit
  • Parser: the fixed five-##/three-$$ header assumption desynchronized on PV5.1 files (two $$ lines — VisuVersion was silently consumed) and could not read ParaVision 360 files at all: @N*(value) run-length encoded arrays, $$ @vis= comments inside wrapped value blocks, commas inside <> strings (e.g. <Parameter maps T2 relaxation, bg: Otsu.>), enum values stored as sized arrays (( 1 ) + disk_normal_slice_order), and scalar struct values on the parameter line.
  • Scaling: VisuCoreDataSlope/VisuCoreDataOffs may hold one value for all frames or one per frame; per-frame indexing read out of bounds when a single value was stored.
  • Geometry: 2D datasets whose frame groups lack FG_SLICE (e.g. FG_ISA parameter maps) are single-slice; deriving the slice count from identical per-frame positions produced a zero slice spacing. The slice axis now follows the sign of the slice-position step along the orientation's third row, generalizing the previous coronal-only Y-component heuristic to oblique stacks.
Test results
  • Existing itkBruker2dseq_PV5.1_FSE_INT16 / PV6.0_FLASH_* regression tests pass with unchanged baselines.
  • New Bruker2dseqImageIO.ReadParaVision360Dataset GTest covers RLE arrays, wrapped strings with embedded commas, mid-value comments, broadcast scaling, frame-group reordering, and a reversed slice axis.
  • Local sweep over 1636 public ParaVision datasets (PV5.1, PV6.0.1, PV7, PV360 3.4-3.7 studies from Zenodo, bruker2nifti_qa, MRIReco.jl, and Bruker's PV360 standard protocols): previously 677 readable, now 1631; the 5 remaining failures are zero-byte placeholder files, rejected with a clean exception. Of the 677 previously readable, 610 outputs are byte-identical; the 67 that changed are slice-axis sign corrections on oblique/coronal 2D stacks, verified against the stored VisuCorePosition progression.
AI assistance
  • Tool: Claude Code
  • Role: implemented the parser rewrite and geometry fixes against the Bruker ParaVision file-format specification (derived from Bruker's D01/D12 File Formats manuals and ParaVision headers), and ran the dataset sweep above.
  • All code was reviewed, built, and tested locally before committing.

@github-actions github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:IO Issues affecting the IO module labels Aug 11, 2026
@gdevenyi

Copy link
Copy Markdown
Contributor Author

This work is built on https://github.com/gdevenyi/brkraw-legacy/blob/main/FILE_FORMAT.md which was constructed using an extensive AI deep dive into publicly available Bruker datasets, the Bruker Paravision manuals over multiple versions.

@dzenanz

dzenanz commented Aug 11, 2026

Copy link
Copy Markdown
Member

@greptileai review this.

@greptile-apps

greptile-apps Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The Bruker reader now supports ParaVision 360 parameter forms, frame scaling, and frame-derived geometry. The previously reported resource-exhaustion path was disproved: the exact oversized run-length input is rejected with an ITK exception before expansion.

Confidence Score: 5/5

No blocking failure remains.

Focused public-reader tests passed for ParaVision 360 parsing, scaling, frame ordering, geometry, and the exact oversized run-length input.

T-Rex T-Rex Logs

What T-Rex did

  • An earlier run of the same harness recorded the precise Bruker JCAMP-DX RLE exception text and showed no bug evidenced by execution, occurring before the expansion loop.
  • Rebuilt the focused Bruker reader test target and ran an authored public-reader test that writes @2147483647*(2) into VisuCoreDataSlope before calling ImageFileReader::Update() with Bruker2dseqImageIO; the test passed after catching the expected Bruker JCAMP-DX RLE ITK exception Bruker JCAMPDX RLE count out of range: @2147483647*(2).
  • Ran the focused ParaVision 360 reader test, which passed while covering parsing, scaling, frame ordering, and geometry.

T-Rex Ran code and verified through T-Rex

Reviews (2): Last reviewed commit: "BUG: Bound Bruker JCAMP-DX run-length ex..." | Re-trigger Greptile

Comment thread Modules/IO/Bruker/src/itkBruker2dseqImageIO.cxx Outdated
PV5.1 headers have two $$ lines, not three, which desynchronized the
fixed-header parse. ParaVision 360 files add run-length encoded
arrays (@n*(value)), $$ comments inside wrapped value blocks, commas
inside <> strings, enum values stored as sized arrays, and scalar
struct values on the parameter line. Parse records by their layout
instead of assuming a fixed header and comma-splittable structs.

Change-Id: I20430e5c667057d20920a9ad9b3c7a85163eb808
VisuCoreDataSlope and VisuCoreDataOffs may be absent, hold a single
value applying to every frame, or hold one value per frame; indexing
them per-frame read out of bounds when a single value was stored.

Change-Id: I9ad4eebf2d9316a0557c93261cd9d2db6f178b36
2D datasets without an FG_SLICE frame group (FG_ISA parameter maps)
are single slice; deriving the slice count from the identical
per-frame positions gave a zero slice spacing. Orient the slice axis
along the actual slice-position step so oblique and coronal stacks
match their stored geometry.

Change-Id: If0dce54d2d8ebd770e85801be8e9d889626521f5
Change-Id: I6caa1598d92f3a51dc4002520cb064aec3ae26bf
KWStyle reports "{ value };" initializers as an unnecessary
semicolon, failing ITKIOBrukerKWStyleTest.

Change-Id: Id2fa7f90374d2b9043413909640517530d524b2a
A crafted repetition count such as @2147483647*(2) in visu_pars
drove a multi-GiB allocation while reading image information.
Reject counts of more than nine digits and expansions past 64 MiB.

Change-Id: I4c0c1e90045173c439febad47f9a40e225f87433
@gdevenyi
gdevenyi force-pushed the bruker-2dseq-format-conformance branch from 350c966 to bc6224d Compare August 11, 2026 15:28
@gdevenyi
gdevenyi marked this pull request as ready for review August 26, 2026 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:IO Issues affecting the IO module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants