pypulseq icon indicating copy to clipboard operation
pypulseq copied to clipboard

Reworked `check_timing` to provide a structured error report

Open FrankZijlstra opened this issue 5 months ago • 5 comments

As per title:

  • check_timing now provides a list of SimpleNamespace error descriptions, e.g. namespace(block=2, event='block', field='duration', error_type='BLOCK_DURATION_MISMATCH', value=0.00102, duration=0.001)
  • The default signature for check_timing remains the same to not break existing code. It will now just print the namespace objects instead the error messages.
  • The error report can be pretty-printed with the print_error_report function provide in check_timing.py.
  • check_timing now accepts an additional argument print_errors=True to directly print the error report if there are errors.
  • Sequence.write now performs a check_timing check by default (can be disabled with check_timing=False) and warns if any errors are found.
  • The "TotalDuration" sequence definition calculation is now moved to Sequence.write
  • Added a unit test for the new check_timing
  • Fixed check_timing to not assume rf_raster_time == adc_raster_time (#191).

In addition, the final commit in this series adds functionality to trace where sequence blocks and events were first created, which I believe helps a lot with identifying where the timing errors originate:

  • This can be enabled/disabled with pp.enable_trace(depth=1) and pp.disable_trace(). It is disabled by default.
  • When traces are available, the check_timing error report will automatically include them.
  • All gradient, RF, and ADC events now include a event.trace field if tracing is enabled. This is the call stack (up to the specified depth) from where the event was created.

Some smaller changes included in this PR:

  • Added warnings for RF and ADC delays smaller than the respective dead time, to make users aware that this is automatically increased to the dead time.
  • Removed trailing whitespace in some files
  • Corrected system == None checks to system is None and added proper optional type hints for system (Union[Opts, None]).

Queries:

  • In this printing of the new timing report I added some colours to distinguish the events, errors, and (optional) trace. It can be disabled in print_error_report by providing colored=False. But I wonder what others think, is this desired, and if so, are the colours nice, does it work in all terminals (or at least not break anything), etc.? Try:
import pypulseq as pp
system = pp.Opts(adc_dead_time=10e-6)
pp.enable_trace()
seq = pp.Sequence(system)
adc = pp.make_adc(num_samples=123, duration=1e-3) # <- system not specified
seq.add_block(adc)
seq.check_timing(print_errors=True)

image

  • In check_timing there was a check whether gradients ramped down to zero in the last block. I removed this check because it does not belong in a timing check. Question is, where should it go? Also, it literally only checked the last block, all other gradient checks occur in add_block.
  • Do you like the event tracing changes? It adds some extra code in all make_* functions.

FrankZijlstra avatar Sep 02 '24 15:09 FrankZijlstra