imr-framework / pypulseq

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

Pypulseq ignores BlockDurationRaster Definition in file #156

Closed gabuzi closed 7 months ago

gabuzi commented 7 months ago

Describe the bug I load a pulseq file with a BlockDurationRaster 5e-06 definition. The resulting sequence has the default value of 1e-5, originating from the __init__() of the Opts class. This makes all timings wrong.

To Reproduce

import pypulseq as pp
# the default block_duration_raster is 10e-6, we use 5e-6 here.
sys_nondefault = pp.Opts(block_duration_raster=5e-6)

# init a seq with nondefault block raster
seq = pp.Sequence(system=sys_nondefault)

# add two identical trapezoid pulses to it. We know they should be 1.5ms in total (rise + flat + fall)
trpz = pp.make_trapezoid("x", rise_time=500e-6, flat_time=500e-6, fall_time=500e-6, amplitude=1)
seq.add_block(trpz)
seq.add_block(trpz)
seq.block_durations
# gives [0.0015, 0.0015]  # correct

# write it to file
seq.write("pypulseq_bug.seq") 

# read it back into a sequence with default blockdur
seq_read_default_opts = pp.Sequence()
seq_read_default_opts.read("pypulseq_bug.seq")
seq_read_default_opts.block_durations
# gives [0.003, 0.003]  # that's wrong!

# read it into a sequence with the right blockdur
seq_read_manual_right_block_durations = pp.Sequence(system=sys_nondefault)
seq_read_manual_right_block_durations.read("pypulseq_bug.seq")
seq_read_manual_right_block_durations.block_durations
# gives [0.0015, 0.0015]  (correct)

Expected behavior Writing a sequence object to file and reading it back should always work to result in the exact same object.

This is with the dev branch. The culprit is here: https://github.com/imr-framework/pypulseq/blob/90c84ed7b09700a59da4c6bbe500b573296d7ce2/pypulseq/Sequence/read_seq.py#L85

as far as I can see in the code, self.block_events is a dict from blockID -> the data in the blocks section. It's thus probably safe to just change the code to take the value from self.definitions. In the reading code, nothing other than that is ever written to it, and it is cleared at the start. This works for the example I have provided.

I will open a PR fixing this shortly.

We can add a bunch of construct -> write-file -> read-file -> compare tests to the test suite.

edited to add: this wasn't actually with the latest dev revision, but the problem is still there with 90c84ed7b0