Implemented rotate3D. Fix for rotate.

[ghost]

Implemented rotate3D. Changed rotate to have the correct rotation direction when rotating around the y-axis. The direction for rotating around the x- and z-axis is not changed.

[github-actions]

Coverage Report

File	Stmts	Miss	Cover	Missing
/home/runner/.local/lib/python3.12/site-packages/pypulseq
add_gradients.py	123	24	80%	44, 52, 61, 85, 92, 120–123, 130–131, 150, 157, 175–206, 211, 239
add_ramps.py	36	36	0%	1–89
align.py	35	4	89%	41, 45, 69, 73
calc_duration.py	25	1	96%	37
calc_ramp.py	218	214	2%	45–353
calc_rf_bandwidth.py	27	20	26%	37–59, 63–67
check_timing.py	87	27	69%	72, 76, 101, 194, 201, 211–255
compress_shape.py	30	1	97%	28
convert.py	40	8	80%	42, 48, 66, 72–73, 82, 88–89
event_lib.py	96	14	85%	6–9, 48–51, 70–71, 205–210
make_adc.py	92	13	86%	63, 72–76, 79, 128, 131, 135, 141, 145, 184, 186, 188, 196
make_adiabatic_pulse.py	129	39	70%	196–200, 217–221, 229–230, 253, 259, 328–347, 451–460, 498–506
make_arbitrary_grad.py	37	7	81%	68, 71, 74, 77, 81, 83, 103
make_arbitrary_rf.py	66	55	17%	83–160
make_block_pulse.py	46	3	93%	112–116, 119
make_delay.py	9	1	89%	27
make_digital_output_pulse.py	16	2	88%	39, 47
make_extended_trapezoid.py	56	12	79%	67, 70, 76, 82, 85, 88, 91, 94, 116, 134, 136, 139
make_extended_trapezoid_area.py	93	3	97%	52, 227, 230
make_gauss_pulse.py	69	20	71%	127–131, 134–158, 165, 168
make_label.py	19	3	84%	36, 41, 43
make_sigpy_pulse.py	116	31	73%	12–13, 112, 115, 119, 154, 157–161, 165, 168–169, 172–173, 188, 195, 200, 212, 215, 240–250, 264, 267, 297–307
make_sinc_pulse.py	68	10	85%	94, 100, 127–131, 135, 138–139, 142–143, 165
make_trapezoid.py	111	7	94%	177, 190, 196, 214, 232, 237, 255
make_trigger.py	16	2	88%	44, 52
opts.py	66	9	86%	78, 83, 102, 142, 166–170
points_to_waveform.py	9	1	89%	27
rotate3D.py	54	6	89%	44, 54, 57, 59, 93–94
rotate.py	76	5	93%	59, 83, 126, 133–134
scale_grad.py	14	1	93%	33
sigpy_pulse_opts.py	26	7	73%	34–41
split_gradient.py	39	31	21%	46–103
split_gradient_at.py	70	27	61%	63–90, 110, 114, 118–120, 154–156
traj_to_grad.py	13	9	31%	26–40
/home/runner/.local/lib/python3.12/site-packages/pypulseq/SAR
SAR_calc.py	113	98	13%	33–40, 55–62, 89–108, 129–132, 168–212, 242–246, 264–306
/home/runner/.local/lib/python3.12/site-packages/pypulseq/Sequence
block.py	368	32	91%	59, 62, 70, 76, 91, 99, 105, 116, 119, 122, 130, 135, 144, 196, 245, 249, 265, 305–308, 337–338, 404, 410, 430, 499, 535, 541, 568, 606, 640
calc_grad_spectrum.py	81	76	6%	68–190
calc_pns.py	40	31	22%	45–96
ext_test_report.py	140	11	92%	23, 58, 61, 135, 227–233
install.py	75	42	44%	31, 52, 69, 71, 112–131, 148, 181–184, 200–212, 254–278
parula.py	4	2	50%	19–86
read_seq.py	312	68	78%	42–43, 90, 93, 105, 110, 116, 123, 132, 141, 146, 149, 157–159, 192, 197, 205–254, 284–287, 302–303, 332–349, 412, 415, 450, 458, 532, 574–578
sequence.py	737	224	70%	11–14, 101–111, 132–145, 183, 248–251, 298, 325, 342, 390, 418, 445–450, 487, 503, 594, 616, 657–660, 714, 752, 763–764, 770, 781, 787, 789, 797, 830–838, 859–881, 924, 926, 929, 955–956, 959–962, 998–1008, 1017–1019, 1063–1075, 1090–1091, 1127–1128, 1154, 1160, 1163, 1200, 1321–1334, 1357, 1385, 1407–1409, 1430, 1461, 1472–1485, 1497–1508, 1554–1555, 1564–1582, 1606, 1636–1644, 1676–1786, 1815, 1829–1839, 1851
write_seq.py	172	8	95%	41, 65, 68–75
/home/runner/.local/lib/python3.12/site-packages/pypulseq/utils
cumsum.py	14	1	93%	17
safe_pns_prediction.py	126	113	10%	50–87, 102–189, 197–214, 222, 244–250, 279–286, 310–336, 344–383, 396–411, 415
tracing.py	16	6	62%	33–34, 42, 54–55, 75
/home/runner/.local/lib/python3.12/site-packages/pypulseq/utils/siemens
asc_to_hw.py	58	53	9%	21–28, 48–106
readasc.py	48	45	6%	25–100
TOTAL	4434	1463	67%

Tests	Skipped	Failures	Errors	Time
1324	18 💤	6 ❌	0 🔥	3m 6s ⏱️

[schuenke]

@BenWilhelmUniklinikFreiburg All source code needs to be in src/pypulseq. So you have to move the rotate3D.py file in order for the code to work and the tests to run.

[ghost]

@schuenke sorry, not sure how that happened. The files should be in the correct place now.

[schuenke]

@BenWilhelmUniklinikFreiburg I had a look at the tests and realized that you were only checking for val_a - val_b > tol without using abs() first, which means that the condition was never true for val_a beeing smaller than val_b. When fixing it, I realized that the tests failed.

For easier debugging, I introduced pytest.mark.parametrize and splitted the one test function with 3 assert statements into 3 test functions with 1 assert statement, each. Now it is obvious, that only the case where you provide the doubled rotation matrix fails.

Maybe you can check yourself what causes the problem. At least in one case the grad.last attribute was the problem.

[schuenke]

Ah, and it's NOT failing for all cases. All multiples of pi/2 work fine 😄

[FrankZijlstra]

Just a note: the Approx class in test_sequence.py should be able to handle checking the equality of arbitrary objects, as an extension of pytest.approx. So it can be used for checking the equality of gradient objects with tolerances. No need to write a custom compare_gradients function for each test script. Previously the Approx class was in a separate utils file in the tests folder, where I think it should be.

E.g. checking the equality of blocks works like this:

pypulseq/tests/test_sequence.py

Line 319 in 1c623fe

assert block_compare == Approx(block_orig, abs=1e-5, rel=1e-5), f'Block {block_counter} does not match'

[mcencini]

It seems to me that tests fails for pypulseq.make_extended_trapezoid('y', [0, 5, 1, 3], convert_to_arbitrary=True, times=[1, 3, 4, 7]) when angles are 0.1, 1.0, 60.0, 400.1, -0.1 and -1.0 and rotation axis is x. It seems that rotation via rotate3D by the composite rotation_matrix @ rotation_matrix generate waveforms along y and z with the same area as the original y waveform. Maybe there is some rounding issues of the scaling factors given by the composite matrix which do not arise when applying two separate rotations - I am trying to investigate and will open a PR to @BenWilhelmUniklinikFreiburg's fork if I manage to fix this.

[mcencini]

Ok, scaling the area together with the other parameters in scale_grad fixes the inconsistency:

scaled_grad = copy(grad)
if scaled_grad.type == 'trap':
    scaled_grad.amplitude = scaled_grad.amplitude * scale
    scaled_grad.flat_area = scaled_grad.flat_area * scale
else:
    scaled_grad.waveform = scaled_grad.waveform * scale
    scaled_grad.first = scaled_grad.first * scale
    scaled_grad.last = scaled_grad.last * scale
scaled_grad.area = scaled_grad.area * scale

I think this is correct, isn't it? After all, it seems that area is a parameter for arbitrary grad events - in other functions such as add_gradients, area is updated as well.

[mcencini]

I opened a PR to @BenWilhelmUniklinikFreiburg fork with my changes - if merged, it should update this PR as well.

It also refactors tests so that it uses existing Approx, now moved to conftest.py to be re-used in different tests, as suggested by @FrankZijlstra.

As Approx was quite slow for test_rotation3D_vs_rotation, I refactored to speed it up. I later noticed that maybe test_rotation3D_vs_rotation generates unecessary long arbitrary waveforms (60000 points) - perhaps that is the reason for slow test runtime? If you like the older Approx version better, we can try reduce waveforms length and see if this is enough to achieve reasonable test runtime.