imr-framework / pypulseq

Pulseq in Python
https://pypulseq.readthedocs.io
GNU Affero General Public License v3.0
125 stars 67 forks source link

block duration rounding assertion test #185

Closed L2roche closed 1 week ago

L2roche commented 4 months ago

Describe the bug When running example sequences such as write_3Dt1_mprage.py, an error arises: ...write_seq.py", line 92, in write assert abs(block_duration_rounded - block_duration) < 1e-6 indeed, an event lasts 13585.5 [seq.add_block(pp.make_delay(delay_TI))] line 92 of the write function in the write_seq.py file

To Reproduce

Expected behavior This behavior is expected given the code. However, the assertion test that comes after having computed the block duration in units of raster duration. Shouldn't this line just be "assert abs(block_duration_rounded - block_duration) <= 0.5" The 'error' also comes from the fact that make_delay does not check for system.grad_raster_time. Another patch could be to make sure beforehand that TI is adjusted such that delay_TI is a multiple of system.grad_raster_time [delay_TI = TI - pp.calc_duration(rf_prep) / 2 - pp.calc_duration(gx_spoil)]

FrankZijlstra commented 4 months ago

Shouldn't this line just be "assert abs(block_duration_rounded - block_duration) <= 0.5"

No, the block duration needs to be an exact multiple of the block_duration_raster specified in the system object. This statement would simply always be True.

Another patch could be to make sure beforehand that TI is adjusted such that delay_TI is a multiple of system.grad_raster_time

This is the correct solution, using this scheme: np.ceil(x * raster_time) / raster_time. Many examples use it, but also many examples are outdated and not using best practices, which is a problem...