imr-framework / pypulseq

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

Fixed `set_block` to give errors when multiple events of the same type are specified #141

Closed FrankZijlstra closed 6 months ago

FrankZijlstra commented 8 months ago

As title. There are two options here:

  1. Only first commit: Give errors for all event types
  2. Both commits: Give errors for RF/ADC events, but automatically add gradients together

Personally I'm in favour of option 2, because it seems like pretty natural behaviour, and it can come up in some instances: e.g. specifying a slab-selection refocusing gradient along with a phase-encoding gradient in 3D imaging. An argument against this behaviour is that it hides something the user should ideally be aware of, and you may end up with weird gradient shapes if the timing of gradients is not aligned.

sravan953 commented 8 months ago

Thanks for the PR!

I'm in favour of option 1 because of the disadvantage with option 2 you mentioned. It seems like coming across an error about multiple gradients and then manually adding gradients is safer than passing multiple gradients and spending time debugging because the output looks different.

FrankZijlstra commented 7 months ago

I reverted the second commit so at least the basic fix can be pushed!

btasdelen commented 6 months ago

I feel like there is no disagreement on the basic fix, so merging.