Skip to content

Add rotate_3d implementation - #378

Merged
schuenke merged 12 commits into
pulseq:masterfrom
Moji3h:PyPulseq_with_rotate3D
Oct 29, 2026
Merged

schuenke merged 12 commits into
pulseq:masterfrom
Moji3h:PyPulseq_with_rotate3D

Conversation

@Moji3h

@Moji3h Moji3h commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@Moji3h

Moji3h commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

This PR adds the rotate3D function to PyPulseq which enables rotation of gradient waveforms in 3D space.

@mcencini mcencini 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.

Thanks @Moji3h for this PR! I left some comments about styling and a suggestion to avoid a custom quat_to_matrix function.

In addition, I think we should add a test suite for this.

Comment thread src/pypulseq/rotate3D.py Resolved
Comment thread src/pypulseq/rotate3D.py Resolved
Comment thread src/pypulseq/rotate3D.py Resolved
@fzimmermann89

Copy link
Copy Markdown
Collaborator

why do the gradients need to be realigned after scaling?

also there are some strange (llm?) comments left in...

@github-actions

github-actions Bot commented Mar 9, 2026 •

Copy link
Copy Markdown

Coverage

Coverage Report

Warning: Comment truncated due to GitHub's 65536 character limit

FileStmtsMissCoverMissing
/home/runner/.local/lib/python3.12/site-packages/pypulseq
   add_gradients.py1372185%44, 52, 61, 85, 92, 125–128, 155, 162, 177, 187–190, 211–218, 225, 230, 261
   add_ramps.py36360%1–92
   align.py35489%41, 45, 69, 73
   calc_duration.py33294%43, 50
   calc_ramp.py2202162%48–359
   calc_rf_bandwidth.py372824%45–81, 85–89
   check_timing.py962970%78, 82, 107, 180, 199, 232, 239, 249–293
   compress_shape.py30197%28
   convert.py40880%42, 48, 66, 72–73, 82, 88–89
   event_lib.py961584%6–9, 48–51, 70–71, 185, 205–210
   make_adc.py981486%77, 80, 90–94, 97, 146, 149, 153, 159, 163, 202, 204, 206, 214
   make_adiabatic_pulse.py1323970%208–212, 232–236, 244–245, 268, 274, 343–362, 466–475, 513–521
   make_arbitrary_grad.py53492%80, 109, 118, 130
   make_arbitrary_rf.py756316%95–179
   make_block_pulse.py48198%128
   make_delay.py9189%27
   make_digital_output_pulse.py16288%42, 50
   make_extended_trapezoid.py561279%67, 70, 76, 82, 85, 88, 91, 94, 116, 134, 136, 139
   make_extended_trapezoid_area.py104496%68, 71, 261, 264
   make_gauss_pulse.py77594%171, 179–181, 196
   make_label.py21290%68, 70
   make_sigpy_pulse.py1193075%12–13, 122, 125, 129, 166–170, 174, 177–178, 181–182, 197, 204, 209, 221, 224, 249–259, 273, 276, 306–316
   make_sinc_pulse.py74593%110, 175–177, 192
   make_soft_delay.py26292%107, 125
   make_trapezoid.py111794%177, 190, 196, 214, 232, 237, 255
   make_trigger.py16288%47, 55
   opts.py66986%78, 83, 102, 142, 166–170
   points_to_waveform.py9189%27
   rotate.py69593%55, 69, 112, 119–120
   rotate_3d.py78297%145–146
   sigpy_pulse_opts.py26773%34–41
   split_gradient.py393121%46–103
   split_gradient_at.py702761%63–90, 110, 114, 118–120, 154–156
   traj_to_grad.py13931%26–40
/home/runner/.local/lib/python3.12/site-packages/pypulseq/SAR
   SAR_calc.py5180%9
/home/runner/.local/lib/python3.12/site-packages/pypulseq/Sequence
   block.py4525488%64, 67, 75, 81, 96, 104, 110, 121, 124, 127, 135, 140, 149, 160, 168, 208, 210, 214, 329, 333, 336, 357, 372, 382, 393, 402, 434, 438, 447, 460, 471, 480–484, 502, 534–541, 558, 568, 594, 632, 671, 685, 712, 791, 828, 852
   calc_grad_spectrum.py81766%68–190
   calc_pns.py403122%45–96
   ext_grad_check.py55296%51, 53
   ext_test_report.py1671193%79, 156, 167–168, 337–343
   install.py754244%31, 52, 69, 71, 112–131, 148, 181–184, 200–212, 254–278
   parula.py4250%19–86
   read_seq.py4084190%44–45, 110, 117, 130, 133–134, 138, 184, 223–224, 242, 415, 436–453, 516, 519, 604, 628, 668, 699, 715–719, 726, 837, 848
   sequence.py64113379%10–13, 107–117, 138–151, 205, 270–273, 320, 364, 440, 467–472, 509, 537–540, 631, 659–668, 680, 702, 743–746, 775, 801–807, 842, 853–854, 860, 871, 877, 879, 887, 920–928, 1060, 1158, 1164, 1167, 1170, 1209, 1334–1347, 1405, 1427–1429, 1450, 1513, 1521, 1619, 1630–1643, 1712–1713, 1724–1742, 1766, 1796–1804, 1840, 1854–1864
   write_seq.py362798%45, 77–79, 313, 347–349, 499
/home/runner/.local/lib/python3.12/site-packages/pypulseq/utils
   cumsum.py14193%17
   paper_plot.py63588%49–132

@Moji3h

Moji3h commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the comments. I updated it accordingly and resolved the issues.

Comment thread src/pypulseq/rotate3D.py Resolved
Comment thread src/pypulseq/rotate3D.py Resolved
@mcencini

Copy link
Copy Markdown
Collaborator

@Moji3h thank you for the review! I still see some issues with the docstring, as well as the remaining issue of post rotation gradient alignment.

In addition, I would strongly advise to add some unit tests, it is important not to decrease the coverage. Thanks again for your support!

@mcencini

mcencini commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Hi @Moji3h, do you have any updates by any chance? I am happy to help with the coverage if you're ok with me pushing commits on this branch

@schuenke

Copy link
Copy Markdown
Collaborator

based on this PR, #302 and https://github.com/m-a-x-i-m-z/pypulseq-matlab-like/blob/master/src/pypulseq_matlab_like/rotate_3d.py me and my AI coding partner created an updated, documented and tested (96% coverage) version of the rotate_3d function.

I also did a MATLAB cross-check using 7 cases covering all input forms and gradient types. All output gradients match in channel, type, timing and waveform (max |Δ| ≈ 1e-10 Hz/m) and trapezoid amplitudes.

I also adjusted the function name (and filename) to rotate_3d with the following API:

def rotate_3d(
    *args: SimpleNamespace,
    rotation: Union[np.ndarray, list, tuple, float, Rotation],
    system: Union[Opts, None] = None,
) -> List[SimpleNamespace]:

So the overlap with the current code is quite small.

@mcencini I guess you would be the best candidate to review it. Do you prefer a new PR or should I push the changes here?

@mcencini

Copy link
Copy Markdown
Collaborator

@schuenke thanks for looking into this! I will be happy to review it asap. I think it is ok to push the changes here.

@schuenke schuenke changed the title Add rotate3D implementation Add rotate_3d implementation Oct 14, 2026

@mcencini mcencini 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.

@schuenke looks good to me!

@schuenke
schuenke merged commit e66547d into pulseq:master Oct 29, 2026
4 checks passed
@schuenke schuenke mentioned this pull request Oct 29, 2026
3 of 8 tasks
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.

4 participants