[fix & feature] MNI coordinate-transform bugs, MI/MItc/max-pressure safety reporting - #129
Open
sirmrmarty wants to merge 4 commits into
Open
[fix & feature] MNI coordinate-transform bugs, MI/MItc/max-pressure safety reporting#129sirmrmarty wants to merge 4 commits into
sirmrmarty wants to merge 4 commits into
Conversation
… helpers Fixes found and independently verified during an audit of the transducer-to-MNI coordinate pipeline: - neuronav_convert_native_to_MNI: the loop that nudges a transducer coordinate 1 mm at a time to fit the MNI FOV had no iteration cap and re-attempted the transform with the *unshifted* coordinate before applying the shift, wasting a subject2mni_coords subprocess call every pass. If the coordinate's X was ever exactly 0, neither shift branch fired, guaranteeing an infinite loop. Now capped at 20 iterations with a clear error on overflow, shifts before retrying, and the >= 0 boundary is closed. - neuronav_convert_native_to_MNI: replaced hand-rolled affine decomposition with the shared transform_coordinates helper, per doc_coordinate_systems.md's own rule against reimplementing affine inversion elsewhere. The manual version was algebraically correct but duplicated existing logic. - convert_final_to_MNI_matlab: the arguments block declared inv_final_transformation_matrix as a plain (4,3) matrix, but the body accessed .tdata.T (struct-style). The one real producer of this matrix (preproc_head.m) is a plain 4x4 double, and this function has no callers anywhere in the repo. Fixed the declaration to (4,4) double and removed the incompatible struct access. - subject2mni_coords_LDfix: input/output were named coords_mni/coords_sub despite the function converting subject -> MNI (backwards from the names). Renamed to coords_subject/coords_mni to match direction, and fixed the assert message (same message typo fixed in mni2subject_coords_LDfix, which was correctly named but had the same copy-pasted message). - doc_coordinate_systems.md: corrected a claim that MNI world-mm coordinates use LAS convention. Verified empirically against a labeled left-hemisphere target output (sub-601 dcPUT): its peak-intensity voxel transforms to X = -69.0 mm via the plain sform, correctly on the left. Only the template's internal voxel storage order is LAS; world-mm output was already correct.
Per-subject and group HTML reports gain a clearer, medium-agnostic mechanical- index view plus a new peak-pressure check, as part of the broader report system in generate_group_report.m / generate_simulation_report.m: - acoustic_analysis now computes a global MI for every simulation (not just layered ones) and a global Psptp for layered runs, mirroring the existing global Isppa/Psptp computation. get_risk_limits defines MI (free water, limit 1.9) and Max pressure (limit 2 MPa) for both media. - Safety dashboards (per-subject and group) show both MI (free water) and MItc (transcranial) tiles side by side instead of the previous per-tissue four-tile breakdown (MI_brain/MI_skull/MI_skin/MI_tc) on that board. Whichever variant has no data for the report's medium renders grayed out as N/A automatically, via the existing missing-data styling. Per-tissue MI values remain available in the CSV and the Mechanical Index box plot. - New Max pressure safety tile (<= 2 MPa) alongside the MI tiles. - generate_group_report's outerjoin_tables now warns (generate_group_report:columnTypeMismatch) when a column's type differs across per-subject CSVs being merged (e.g. mixed-version CSVs in the same path.sim), instead of silently coercing or dropping values. - doc_group.md / doc_outputs.md updated: box plots auto-scale figure width to the number of plotted columns and export as 150 dpi PNGs; NSR limits table and CHANGELOG updated for the new MI/Max-pressure metrics.
New placement.mode = 'mni', alongside the existing manual/localite/heuristic/ plantus modes: specify both the transducer scalp-entry point and the focus directly in MNI mm, with no search - deterministic placement. - Focus converts via the nonlinear SimNIBS warp (identical to the heuristic target path); the transducer point converts via the linear 12dof affine instead, since the nonlinear warp is unreliable outside the brain. - Converted transducer point snaps to the nearest scalp voxel, then the same standoff-geometry offset the heuristic already applies to every candidate position is applied here too. - Extracted two shared helpers so both placement paths use one implementation instead of duplicating the geometry math: tp_scalp_boundary (scalp/outer- boundary voxel extraction, factored out of tp_candidate_mesh) and transducer_scalp_geometry (standoff-geometry math, same factoring). tp_candidate_mesh now calls these helpers instead of its old inline code; behaviour is unchanged there (verbatim extraction). - New config.placement.mni block (trans_pos_mm, focus_pos_mm, skin_gap_mm) and doc_placement_mni.md documenting the new mode.
…alysis acoustic_analysis.m (already committed) calls compute_tic(), which had never been committed - a fresh checkout would crash with "Undefined function 'compute_tic'" on every acoustic simulation, not just fail to build a report. This commit adds that function along with the rest of the report-system work it was extracted alongside, since generate_simulation_report.m / generate_group_report.m (also already committed) call into the other pieces here too (report_scripts, build_config_dump, new html_utils methods) - those calls are guarded by a try/catch so they only degraded to "no report file produced", but the whole redesign belongs together as one unit: - compute_tic: IEC 62359 Cranial Thermal Index. The one modeling assumption (emitted power estimated from peak Isppa x aperture area x duty cycle) is isolated in its own local function and documented as an upper bound. - report_scripts: shared <script> blocks (theme toggle, in-page search, scroll-spy, lightbox zoom/compare, sortable roster, client-side distribution/correlation/by-condition charts) used by both report generators instead of inline JS. - build_config_dump: renders the parameters struct as a searchable, escaped, size-capped HTML block. - css_styles_base: visual redesign (light "paper instrument" / dark "control room" theme via data-theme) with the classes the report generators emit. - html_utils: adds embed_image_dims (per-slice vs single-image detection) and report_logo (inlines the vendored logo as base64); reworks lightbox markup to match report_scripts. - assets/logo_PRESTUS.png: vendored logo consumed by report_logo. - thermal_analysis: adds riseT37_brain/skull/skin (temperature rise from a fixed 37C baseline), which get_risk_limits/test_reports already expect. - run_all_tests, test_reports: registers and adds a new suite exercising all of the above end-to-end (per-subject and group report generation, TIC, config dump, image-dims detection) against synthetic data.
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
1 participant
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.
MNI coordinate for transducer positioning: generalise across participants' transducer positions by allowing the definition of the position in
MNIMI (free water) and Max pressure metrics: acoustic_analysis now computes a global MI for every simulation (not just layered) and a global Psptp for layered runs, mirroring the existing global Isppa computation. get_risk_limits adds MI (limit 1.9) and Max pressure (limit 2 MPa) for both media.
Safety dashboard MI/MItc tiles: per-subject and group HTML reports now show both MI (free water) and MItc (transcranial) tiles side by side, instead of the previous per-tissue breakdown (MI_brain/MI_skull/MI_skin/MI_tc). Whichever variant has no data for the report's medium renders grayed out as N/A automatically. Per-tissue values remain in the CSV and the existing Mechanical Index box plot.
outerjoin_tables type-mismatch warning: the group report's table merge now warns (generate_group_report:columnTypeMismatch) when a column's type differs across per-subject CSVs being combined — e.g. mixed-version CSVs left in the same path.sim — instead of silently coercing or dropping values.