Skip to content

Fix #906: inati should be iterative and walsh should phase align - #993

Open
fzimmermann89 wants to merge 3 commits into
mainfrom
csm
Open

fzimmermann89 wants to merge 3 commits into
mainfrom
csm

Conversation

@fzimmermann89

Copy link
Copy Markdown
Member

This fixes #906
Inati should be the iterative inati method (2014), not walsh + phase alignment.
Also, it adds phase alignment (can be disabled for pure walsh) to walsh, making it inati 2013.

@fzimmermann89
fzimmermann89 requested a review from schuenke May 4, 2026 12:29
@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

Coverage

Coverage Report
FileStmtsMissCoverMissing
src/mrpro
   _version.py6267%7–8
src/mrpro/algorithms/csm
   inati.py26196%76
src/mrpro/algorithms/dcf
   dcf_voronoi.py55493%15, 55–56, 89
src/mrpro/algorithms/optimizers
   adam.py30680%108, 125–129
   cg.py52198%139
   pdhg.py82396%187–188, 194
   pgd.py53492%110, 158–161
src/mrpro/algorithms/reconstruction
   DirectReconstruction.py30293%64, 80
   Reconstruction.py56886%57–59, 123–126, 128, 136
   RegularizedIterativeSENSEReconstruction.py52296%125, 153
   TotalVariationRegularizedReconstruction.py54493%106, 112, 129, 132
src/mrpro/data
   AcqInfo.py165796%49, 56, 134–135, 137, 243, 367
   Dataclass.py3112692%58, 319, 335, 401, 465–467, 480, 575, 595–596, 598, 613–614, 616, 663–664, 669–670, 855–856, 881, 888, 893–894, 896
   DcfData.py33197%62
   EncodingLimits.py97397%37, 127, 130
   IData.py1781293%138, 155–156, 217–225, 258, 304, 322
   IHeader.py174895%49, 98–101, 284, 288, 292, 296
   KData.py2262489%121–122, 137, 144, 155–166, 177, 185, 196, 235, 257–259, 306–307, 372, 538, 540, 612
   KHeader.py1711392%119–125, 152, 200, 207–208, 240–247
   KNoise.py22195%44
   KTrajectory.py95397%163, 165, 185
   Rotation.py7374294%104, 202, 339, 437, 481, 499, 586, 588, 597, 634, 636, 699, 776, 781, 784, 799, 816, 821, 897, 1085, 1090, 1093, 1117, 1121, 1145, 1265, 1267, 1275–1276, 1340, 1422, 1626, 1633–1635, 1694, 1790, 1942, 1977, 1981, 2156, 2177
   SpatialDimension.py1501987%34, 103, 146, 158, 278–280, 293–295, 329, 347, 360, 373, 386, 399, 408–409, 437
src/mrpro/data/traj_calculators
   KTrajectoryCalculator.py26292%84, 95
   KTrajectoryCartesian.py31487%129–132, 136
   KTrajectoryIsmrmrd.py19195%57
   KTrajectorySpiral2D.py571377%63–66, 69, 71, 73, 75, 77, 105, 107, 134–136
src/mrpro/operators
   AveragingOp.py38295%73, 115
   B0InformedFourierOp.py75297%151–154
   CartesianSamplingOp.py112496%152, 191, 266, 387
   ConjugateGradientOp.py89792%62, 64, 100, 106, 228, 230, 233
   ConstraintsOp.py85495%78, 80, 250, 255
   EndomorphOperator.py28293%71, 77
   FiniteDifferenceOp.py29293%40, 126
   FourierOp.py105991%85–87, 192–193, 212, 257, 321, 326
   Functional.py70297%116, 118
   GridSamplingOp.py1651591%72–73, 82–83, 90–91, 94, 96, 98, 282, 290–291, 306, 312–313
   LinearOperator.py202697%217, 255, 296, 305, 313, 330
   LinearOperatorMatrix.py1741989%99, 136, 169, 178, 183, 192–195, 208–211, 219–220, 225–226, 238, 347, 377, 404
   MultiIdentityOp.py16288%58, 63
   NonUniformFastFourierOp.py1981095%72, 99, 221, 223, 261, 263, 340, 397, 471, 476
   Operator.py88397%82, 115, 125
   PatchOp.py49394%93, 129, 144
   ProximableFunctionalSeparableSum.py42393%118, 213, 224
   SliceProjectionOp.py1781094%45, 62, 64, 70, 154, 180, 216, 253, 290, 330
   WaveletOp.py119397%184, 228, 254
   ZeroPadOp.py18194%30
src/mrpro/operators/functionals
   SSIM.py73495%82, 86, 114, 147
src/mrpro/operators/models
   EPG.py2561993%31–32, 283, 288, 304–306, 326–327, 332, 356, 361, 386, 391, 545, 599, 738, 755, 782
src/mrpro/phantoms
   EllipsePhantom.py43295%66, 131
   b0map.py262119%37–63
   brainweb.py2974087%276, 290–294, 349–359, 398, 454–457, 479–480, 485–486, 488–489, 493, 501, 508–509, 550, 621, 624–625, 644, 653–656, 667, 669, 700–701, 715, 723
   fastmri.py1061091%50–51, 59, 65, 162, 169–171, 174–175, 189
   m4raw.py74495%58–59, 74, 76
   mdcnn.py71790%58, 62–63, 70, 82, 88, 135
src/mrpro/utils
   RandomGenerator.py1551590%23–24, 36, 38, 188, 212, 428, 446, 528, 799, 829–832, 895, 898
   ema.py42490%49, 74, 78, 82
   filters.py61198%46
   indexing.py177199%321
   pad_or_crop.py39685%40, 43, 46, 49, 66, 73
   reshape.py1531093%176, 370, 482–484, 505, 508–509, 515, 530
   slice_profiles.py49688%21, 37, 119–122, 155
   split_idx.py10280%43, 47
   summarize.py57689%40–41, 70–73, 81
   unit_conversion.py721579%34, 44, 51, 53, 62, 69, 71, 78, 80, 89, 100, 121, 123, 144, 146
TOTAL811049894% 

Tests Skipped Failures Errors Time
3304 0 💤 0 ❌ 0 🔥 2m 34s ⏱️

@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

📚 Documentation

📁 Download as zip
🔍 View online

@schuenke schuenke left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation, tests and documentation look good to me.

Maybe @ckolbPTB wants to have a quick look as well?!

@fzimmermann89

Copy link
Copy Markdown
Member Author

It would also be great to test it on another challenging dataset.
For the mrf data I did this change for, all three codes result in slightly different csms but unfortunately none seems clearly superior.
Nevertheless, I think this would be the proper csm Walsh and Inati variants to use

@ckolbPTB

Copy link
Copy Markdown
Collaborator

I compared the old and the new methods on a few datasets and all show a similar behaviour as this:

csm

The new Walsh implementation with phase alignment leads to a reasonable homogenous phase.

Nevertheless, the new Inati now leads to the same results as Walsh. I could not find a dataset where there was any noticeable difference and the number of iterations also does not seem to have an effect (i.e. no difference between first and last iteration). The result do not have a flat phase (more pronounced in cardiac datasets) whereas the old implementation leads to a completely constant phase.

I could not find a bug so my suggestion would be to only change the Walsh implementation but leave the old Inati method and clearly label it as non-iterative. @fzimmermann89 @schuenke what are your thoughs on this?

We should really work on the ESPIRT coil estimation...

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.

Inati's method for csm is walsh / inefficient phase correction

3 participants