Read Pulseq 1.5.0

[Stockless]

This PR addresses issue #555 by adding support for reading Pulseq files in version 1.5.0.

[cncastillo]

Please use three correct logic so pulseq 1.4 still works. No need to pass the version into the get_X functions, you can just check the input length of the library.

[codecov]

Codecov Report

❌ Patch coverage is 90.71207% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.05%. Comparing base (b5c63a9) to head (e37125f).
⚠️ Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
KomaMRIFiles/src/Sequence/Pulseq.jl	90.27%	18 Missing ⚠️
KomaMRIBase/src/datatypes/sequence/RF.jl	59.09%	9 Missing ⚠️
KomaMRIBase/src/datatypes/Sequence.jl	83.33%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #614      +/-   ##
==========================================
+ Coverage   90.14%   91.05%   +0.90%     
==========================================
  Files          57       57              
  Lines        3228     3387     +159     
==========================================
+ Hits         2910     3084     +174     
+ Misses        318      303      -15     

Flag	Coverage Δ
base	91.28% <89.83%> (+2.19%)	⬆️
core	89.73% <100.00%> (ø)
files	93.75% <90.90%> (-0.31%)	⬇️
komamri	88.13% <100.00%> (-0.08%)	⬇️
plots	90.90% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
KomaMRIBase/src/KomaMRIBase.jl	100.00% <ø> (ø)
KomaMRIBase/src/datatypes/Phantom.jl	86.01% <100.00%> (+0.14%)	⬆️
KomaMRIBase/src/datatypes/sequence/Grad.jl	94.54% <100.00%> (+5.15%)	⬆️
KomaMRIBase/src/motion/Action.jl	100.00% <100.00%> (+25.00%)	⬆️
KomaMRIBase/src/motion/Motion.jl	100.00% <100.00%> (+7.69%)	⬆️
KomaMRIBase/src/motion/MotionList.jl	100.00% <100.00%> (+7.00%)	⬆️
KomaMRIBase/src/motion/NoMotion.jl	96.00% <100.00%> (ø)
KomaMRIBase/src/motion/TimeCurve.jl	95.83% <100.00%> (+35.83%)	⬆️
KomaMRIBase/src/motion/actions/ArbitraryAction.jl	75.00% <ø> (+8.33%)	⬆️
...se/src/motion/actions/arbitraryactions/FlowPath.jl	100.00% <100.00%> (+75.00%)	⬆️
... and 10 more

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cncastillo]

(1) Update get_events to add compat_helper copy: https://github.com/pulseq/pulseq/blob/master/matlab/%2Bmr/%40Sequence/read.m#L520. get_events cannot be hardcoded for the RF case

(2) Test with reader with (take inspiration from https://juliahealth.org/KomaMRI.jl/dev/explanation/5-seq-events/):

• arbitrary gradients
• time shaped gradients
• trapezoidal gradients
• arbitrary rfs
• time shaped rfs
• block rfs

@Stockless can you give use the MATLAB files you were using for the tests?

(3) Modify to consider new RF uses, to calculate k-space correctly (not guess RF use by > 90.0):

• get_RF_types() → 1st
• get_RF_center() → 2nd
• get_Mk() → 3rd

If use is Undefined(), keep using the current method; otherwise, use the provided value.

If center is not defined (i.e., == 0.0), keep the current method; otherwise, use the provided value.

[gsahonero]

@pvillacorta , @cncastillo mentioned you are interested in working this PR out. I want to get it done too. Let's meet to sort out the efforts?

[pvillacorta]

@pvillacorta , @cncastillo mentioned you are interested in working this PR out. I want to get it done too. Let's meet to sort out the efforts?

Yes! Let's join forces :)

[cncastillo]

@pvillacorta @gsahonero do you guys want to plan a meeting to push this?

[pvillacorta]

@gsahonero and I are meeting this thursday at 4pm (spanish time). I hope we can plan how to finish this PR and the one for writing Pulseq.

[github-actions]

KomaMRI Benchmarks

Details

Benchmark suite	Current: e37125f	Previous: 11d3180	Ratio
MRI Lab/Bloch/CPU/2 thread(s)	336940162.5 ns	337633469 ns	1.00
MRI Lab/Bloch/CPU/4 thread(s)	275131359.5 ns	276282606 ns	1.00
MRI Lab/Bloch/CPU/8 thread(s)	238117403 ns	209852165 ns	1.13
MRI Lab/Bloch/CPU/1 thread(s)	564731030 ns	555065232 ns	1.02
MRI Lab/Bloch/GPU/CUDA	21277447.5 ns	21231134 ns	1.00
MRI Lab/Bloch/GPU/oneAPI	82937542 ns	77053690 ns	1.08
MRI Lab/Bloch/GPU/Metal	93370833 ns	95540333 ns	0.98
MRI Lab/Bloch/GPU/AMDGPU	18731441.5 ns	24763685 ns	0.76
Slice Selection 3D/Bloch/CPU/2 thread(s)	1565922227 ns	1592087059 ns	0.98
Slice Selection 3D/Bloch/CPU/4 thread(s)	877740301 ns	889539516 ns	0.99
Slice Selection 3D/Bloch/CPU/8 thread(s)	558875084 ns	565401528.5 ns	0.99
Slice Selection 3D/Bloch/CPU/1 thread(s)	3058039968 ns	3029269071 ns	1.01
Slice Selection 3D/Bloch/GPU/CUDA	31889354 ns	32639703 ns	0.98
Slice Selection 3D/Bloch/GPU/oneAPI	124090097 ns	121489006.5 ns	1.02
Slice Selection 3D/Bloch/GPU/Metal	108830396 ns	111152750 ns	0.98
Slice Selection 3D/Bloch/GPU/AMDGPU	25108034.5 ns	32636098 ns	0.77

This comment was automatically generated by workflow using github-action-benchmark.

[pvillacorta]

Pulseq files from KomaMRIFiles/test/test_files/pulseq_read_comparison must ve reviewed, since I have not been able to retrieve their signature. For the moment, I have just added a warning when the retrieved signature and the expected one do not coincide (obviously, the ideal case would be to throw an error, but tests would not pass for pulseq_read_comparison).

[pvillacorta]

I will start with tests tomorrow, with this in mind:

(2) Test with reader with (take inspiration from https://juliahealth.org/KomaMRI.jl/dev/explanation/5-seq-events/):
- arbitrary gradients
- time shaped gradients
- trapezoidal gradients
- arbitrary rfs
- time shaped rfs
- block rfs

I will also try to test the reader with .seq files made with write_seq (see this branch, which is sync with this PR and for the moment is in my Koma fork). Once this PR is merged, I will create a new one with the write functions.

[pvillacorta]

For tests, I think we need three complementary testing strategies, and all of them are important for proper validation:

1. Round-trip test inside KomaMRI (for both read_seq and write_pulseq)

• Take a Koma Sequence (call it s1), run write_seq to produce a .seq file, then load it back with read_seq to get s2, and finally check that s1 == s2.
  This verifies internal consistency between writing and reading.
  However, it might not catch certain compatibility issues with other Pulseq interpreters: errors in write_seq could be “undone” by read_seq, so a strict s1 == s2 check doesn’t necessarily guarantee that the generated .seq file fully conforms to the Pulseq 1.5 standard.

2. Cross-checking against sequences generated by MATLAB Pulseq and PyPulseq

• For write_seq: Generate equivalent sequences using the MATLAB and Python implementations of Pulseq, then compare them with the ones generated by KomaMRI.
• For read_seq:
  • Use Koma's read_seq function to read a .seq file produced by MATLAB. Then, compare the fields of the loaded Sequence with the output of the seq.plot() MATLAB function. This is what is actually done for now, but it is pending for v1.5. See this comment to learn how to generate a .mat file from a .seq file.
  • Use Koma's read_seq function to read a .seq file produced by PyPulseq. See Test files generated with Python PyPulseq (1.3) #367.

3.Ensuring MATLAB and PyPulseq can read KomaMRI-generated sequences (write_seq tests)

• Another angle is to test interoperability: generate sequences with KomaMRI, load them in MATLAB Pulseq and PyPulseq, and compare their internal representations or waveform plots.
  If both interpreters can parse and reproduce the expected plots, it’s a strong indication that the KomaMRI writer produces fully standard-compliant .seq files.

Together, these three kinds of tests give good coverage:
(1) internal correctness, (2) external consistency with other implementations, and (3) practical interoperability with the broader Pulseq ecosystem.

[pvillacorta]

I have merged #658 into this PR, and made the center field to be always defined (that is, calculating it directly within the RF constructor in case it is not passed as an input argument). I think this approach makes sense, since, from now, all the fields of an RF instance would have a "correct" value. Otherwise, sometimes we would have rf.center = 0.0, which is a bit confusing. Let me know what you think :)