Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gradient and autodiff checks for linear operators #536

Merged
merged 29 commits into from
Mar 6, 2025

Conversation

ckolbPTB
Copy link
Collaborator

@ckolbPTB ckolbPTB commented Nov 14, 2024

This supersedes #407 - it was too difficult to bring this up-to-date with main.

forward_mode_autodiff_of_linear_operator_test and gradient_of_linear_operator_test still need to be added to

  • ZeroOp
  • RearrangeOp
  • PCACompOp
  • GridSamplingOp

Do we want to run forward_mode_autodiff_of_linear_operator_test and gradient_of_linear_operator_test on all parametrize-options used for the adjointness tests?

Copy link
Contributor

github-actions bot commented Nov 14, 2024

Coverage

Coverage Report
FileStmtsMissCoverMissing
src/mrpro/algorithms/csm
   inati.py24196%44
   walsh.py16194%53
src/mrpro/algorithms/dcf
   dcf_voronoi.py53492%15, 55–56, 89
src/mrpro/algorithms/optimizers
   adam.py20195%101
   pdhg.py79396%177–178, 184
src/mrpro/algorithms/reconstruction
   DirectReconstruction.py281643%59–79, 93
   IterativeSENSEReconstruction.py13192%79
   Reconstruction.py502256%42, 54–56, 80–87, 108–117
   RegularizedIterativeSENSEReconstruction.py411759%97–101, 115–140
src/mrpro/data
   AcqInfo.py128398%26, 169, 207
   CsmData.py29390%15, 84–86
   DcfData.py45882%18, 66, 78–83
   IData.py67987%120, 126, 130, 160–168
   IHeader.py75791%75, 109, 127–131
   KData.py2142588%111–112, 127, 134, 144, 152, 206–207, 245, 250–251, 270–281, 440, 442, 507, 522, 559, 590, 599
   KHeader.py1531789%25, 119–123, 150, 199, 210, 217–218, 221, 228, 260–271
   KNoise.py311552%39–52, 56–61
   KTrajectory.py811285%112–117, 120–122, 207–211
   MoveDataMixin.py1401887%28, 126, 142, 156–158, 220, 336–338, 351, 430, 450–451, 453, 468–469, 471
   QData.py39782%42, 65–73
   Rotation.py6743595%100, 198, 335, 433, 477, 495, 582, 584, 593, 627, 629, 692, 769, 774, 777, 792, 809, 814, 890, 1078, 1083, 1086, 1110, 1114, 1242, 1244, 1252–1253, 1317, 1399, 1692, 1844, 1879, 1883, 1994
   SpatialDimension.py2322191%34, 104, 141, 148, 154, 274–276, 289–291, 325, 343, 356, 369, 382, 395, 404–405, 420, 429
   acq_filters.py12192%47
src/mrpro/data/traj_calculators
   KTrajectoryCalculator.py25292%23, 45
   KTrajectoryIsmrmrd.py13285%41, 50
   KTrajectoryPulseq.py23196%55
src/mrpro/operators
   CartesianSamplingOp.py89397%118, 157, 280
   ConstraintsOp.py60297%46, 48
   EndomorphOperator.py65297%228, 234
   FiniteDifferenceOp.py27293%40, 105
   FourierOp.py158398%274, 392, 397
   Functional.py71593%20–22, 117, 119
   GridSamplingOp.py136993%72–73, 82–83, 90–91, 94, 96, 98
   LinearOperator.py168895%114, 224, 254, 295, 304, 312, 329, 364
   LinearOperatorMatrix.py1581690%82, 119, 152, 161, 166, 175–178, 191–194, 203, 215, 304, 331, 359
   MultiIdentityOp.py13285%43, 48
   Operator.py78297%31, 87
   ProximableFunctionalSeparableSum.py39392%50, 103, 110
   SliceProjectionOp.py173895%44, 61, 63, 69, 207, 228, 261, 301
   WaveletOp.py119596%151, 169, 204, 209, 232
   ZeroPadOp.py16194%30
src/mrpro/utils
   filters.py62297%44, 49
   reshape.py60198%191
   slice_profiles.py46687%20, 36, 113–116, 149
   sliding_window.py34197%34
   split_idx.py10280%43, 47
   summarize_tensorvalues.py11918%20–29
   typing.py211148%8–23
   zero_pad_or_crop.py31681%26, 30, 55, 58, 61, 64
TOTAL497336193% 

Tests Skipped Failures Errors Time
2516 0 💤 0 ❌ 0 🔥 1m 24s ⏱️

Copy link
Contributor

github-actions bot commented Nov 14, 2024

📚 Documentation

📁 Download as zip
🔍 View online

@ckolbPTB
Copy link
Collaborator Author

Tests which currently do not pass:
FAILED tests/operators/test_slice_projection_op.py::test_slice_projection_op_grad - RuntimeError: Sparse CSR tensors do not have strides
FAILED tests/operators/test_slice_projection_op.py::test_slice_projection_op_forward_mode_autodiff - RuntimeError: Sparse CSR tensors do not have strides

@fzimmermann89
Copy link
Member

That is a known issue #409 🙈
pytorch/pytorch#97286 pytorch/pytorch#136357

There is imho no feasibe way tfor us to support the slice projection operator in torch.func.

Fixing it in pytorch would also be infeasible for us, as the the functorch c backend seems to cause the issue, as sparse tensors or completely missing in its implementation,

We could replace the sparse-matrix/dense vector multiplication by manual indexing and dotproducts. This will make it quite a bit slower and way more complicated.

For now, I would ether remove the tests or mark them as failing.

This mainly means that we cant solve a superresolutionproblem with fista or pdgh if we keep using torch.func for the gradients.

@ckolbPTB
Copy link
Collaborator Author

@fzimmermann89 do you think you could add the missing tests for GridSamplingOp? Everthing else should now be covered...

@ckolbPTB ckolbPTB mentioned this pull request Dec 10, 2024
24 tasks
@fzimmermann89
Copy link
Member

I moved #604 out of this PR, to unblock #293

@ckolbPTB ckolbPTB self-assigned this Feb 24, 2025
Copy link
Contributor

github-actions bot commented Mar 2, 2025

Coverage

Coverage Report
FileStmtsMissCoverMissing
src/mrpro
   _version.py6267%7–8
src/mrpro/algorithms/csm
   inati.py24196%44
   walsh.py16194%53
src/mrpro/algorithms/dcf
   dcf_voronoi.py55493%15, 55–56, 89
src/mrpro/algorithms/optimizers
   adam.py20195%101
   pdhg.py79396%177–178, 184
   pgd.py51492%106, 151–154
src/mrpro/algorithms/reconstruction
   DirectReconstruction.py281643%59–79, 93
   IterativeSENSEReconstruction.py13192%79
   Reconstruction.py502256%42, 54–56, 80–87, 108–117
   RegularizedIterativeSENSEReconstruction.py411759%97–101, 115–140
src/mrpro/data
   AcqInfo.py145597%42, 121–122, 124, 233
   CsmData.py29390%15, 84–86
   DcfData.py45882%18, 66, 78–83
   EncodingLimits.py73396%33, 123, 126
   IData.py59886%118, 132, 159–167
   IHeader.py1361291%69–72, 253, 257, 261, 265, 299–303
   KData.py2012986%113–114, 129, 136, 147–157, 166, 174, 183, 217, 239–241, 277–278, 333–344, 473, 475, 545
   KHeader.py1412185%24, 109–115, 142, 190, 197–198, 201, 208, 225–232, 240–251
   KNoise.py311552%39–52, 56–61
   KTrajectory.py108397%166, 168, 188
   MoveDataMixin.py1401887%28, 126, 142, 156–158, 220, 336–338, 351, 430, 450–451, 453, 468–469, 471
   QData.py39782%42, 65–73
   Rotation.py7193595%101, 199, 336, 434, 478, 496, 583, 585, 594, 628, 630, 693, 770, 775, 778, 793, 810, 815, 891, 1079, 1084, 1087, 1111, 1115, 1243, 1245, 1253–1254, 1318, 1400, 1703, 1855, 1890, 1894, 2084
   SpatialDimension.py2322091%34, 104, 148, 154, 274–276, 289–291, 325, 343, 356, 369, 382, 395, 404–405, 420, 429
   acq_filters.py12192%47
src/mrpro/data/traj_calculators
   KTrajectoryCalculator.py27196%84
   KTrajectoryIsmrmrd.py19195%57
src/mrpro/operators
   CartesianSamplingOp.py96496%124, 163, 238, 305
   ConstraintsOp.py60297%46, 48
   EndomorphOperator.py32294%52, 58
   FiniteDifferenceOp.py27293%40, 105
   FourierOp.py90397%185, 257, 262
   Functional.py77988%20–22, 117, 119, 226–228, 242
   GridSamplingOp.py136993%72–73, 82–83, 90–91, 94, 96, 98
   LinearOperator.py201896%107, 217, 244, 251, 292, 301, 309, 326
   LinearOperatorMatrix.py1621988%82, 119, 152, 161, 166, 175–178, 191–194, 202–203, 208–209, 221, 310, 337, 364
   MultiIdentityOp.py13285%43, 48
   NonUniformFastFourierOp.py1881095%69, 96, 206, 208, 241, 243, 319, 373, 423, 428
   Operator.py79297%32, 88
   ProximableFunctionalSeparableSum.py39392%50, 103, 110
   SliceProjectionOp.py174895%45, 62, 64, 70, 206, 227, 260, 300
   WaveletOp.py119596%151, 169, 204, 209, 232
   ZeroPadOp.py16194%30
src/mrpro/phantoms
   brainweb.py2752691%325–335, 371, 427–430, 452–453, 458–459, 474, 520, 586–587, 606–609, 620, 622, 657–658, 671
src/mrpro/utils
   filters.py62297%44, 49
   reshape.py95298%110, 301
   slice_profiles.py47687%21, 37, 116–119, 152
   sliding_window.py34197%34
   split_idx.py10280%43, 47
   summarize_tensorvalues.py12375%23, 25, 27
   typing.py695520%9–235
   unit_conversion.py601477%32, 40, 42, 49, 51, 58, 60, 69, 80, 82, 101, 103, 124, 126
   zero_pad_or_crop.py31681%26, 30, 55, 58, 61, 64
TOTAL569546892% 

Tests Skipped Failures Errors Time
2687 0 💤 0 ❌ 0 🔥 2m 7s ⏱️

Copy link
Member

@fzimmermann89 fzimmermann89 left a comment

Choose a reason for hiding this comment

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

Ignore my previous question about fixture.
I prefer your current style.

For new functions and tests, please add type hints

@ckolbPTB ckolbPTB linked an issue Mar 5, 2025 that may be closed by this pull request
@fzimmermann89 fzimmermann89 self-requested a review March 5, 2025 20:56
@fzimmermann89 fzimmermann89 added the pre-commit.ci autofix run autofix in this PR label Mar 5, 2025
@pre-commit-ci pre-commit-ci bot removed the pre-commit.ci autofix run autofix in this PR label Mar 5, 2025
@fzimmermann89
Copy link
Member

I hope I did the merge correctly...

@fzimmermann89 fzimmermann89 added the pre-commit.ci autofix run autofix in this PR label Mar 5, 2025
@pre-commit-ci pre-commit-ci bot removed the pre-commit.ci autofix run autofix in this PR label Mar 5, 2025
fzimmermann89 and others added 2 commits March 6, 2025 10:14
Co-authored-by: Christoph Kolbitsch <christoph.kolbitsch@ptb.de>
@fzimmermann89 fzimmermann89 enabled auto-merge (squash) March 6, 2025 09:53
@fzimmermann89 fzimmermann89 merged commit 15060bf into main Mar 6, 2025
26 checks passed
@fzimmermann89 fzimmermann89 deleted the gradient_checks_only_for_lin_op branch March 6, 2025 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

additional check for adjoint/derivate of linear operators
2 participants