indetermination when specifying timing
schmidtijoe opened this issue · 3 comments
Describe the bug
Documentation of most functions containing timing (e.g. make_trapezoid) states durations should be given in ms on almost all variables that have anything to do with timing.
However, all of the examples give all of the timings in seconds, e.g. from write_gre.py:
gx = pp.make_trapezoid(channel='x', flat_area=Nx * delta_k, flat_time=3.2e-3, system=system)
I am indecisive on what to do myself since clearly max slew rates etc. need to be calculated accordingly and apart from that i wouldnt want 2 sec gradients running on my scans :)
I switched to the master branch to get the stable version. I suppose this is the one installed via pip. I found in the dev branch this was already changing to s?
To Reproduce
Eg. in docstrings of make_trapezoid, make_gauss_pulse etc.:
delay : float, optional Delay in milliseconds (ms). duration : float, optional, default=0 Duration in milliseconds (ms). flat_time : float, optional, default=-1 Flat duration in milliseconds (ms).
But also in those functions:
if bandwidth == 0: BW = time_bw_product / duration
which would clearly not evaluate a Hz/px bandwidth if computed with a duration in ms
Expected behavior
Concise usage of SI units, in this case s for all functions handling timing, updating of docstrings.
@schmidtijoe Sorry for the mess! Yes, the dev branch should contain more accurate docs. There's one function from Pulseq 1.4.0 that is yet to be included in PyPulseq, which is the only thing holding me back from putting out another release. Hoping to get to it soon!
Ok, I cloned into dev. Thanks for the clarification. It just appears one has to go quite far down many functions to see what is the appropriate input. But anyhow, i hope you find the time for the update! :)
Have been actively working on it... hope to have it released soon 👍🏼