Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pulse Designer #305

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Pulse Designer #305

wants to merge 10 commits into from

Conversation

beorostica
Copy link
Contributor

No description provided.

@@ -0,0 +1,26 @@
"""
"""
function make_adc(num_samles; Δtadc=0, duration=0, delay=0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo num_samles to num_samples

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function will be completelly replaced

delay = dead_time; # adcDeadTime is added before the actual sampling (and also second time after the sampling period)
end

adc = ADC(num_samles, duration, delay, freq_offset, phase_offset)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should output a Sequence

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function will be completelly replaced, all the outpus will be of type Sequence

@assert maximum(abs.(waveform)) <= max_grad "Gradient amplitude violation ($(maximum(abs.(waveform)) / max_grad * 100)%)"

duration = (length(waveform)-1) * Δtgr
return Grad(waveform, duration, 0, 0, delay)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should output a Sequence

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function will be completelly replaced, all the outpus will be of type Sequence

dly = Delay(delay + duration + ring_down_time) # I NEED a review
end

return RF(amplitude, duration, freq_offset, delay), dly
Copy link
Member

@cncastillo cncastillo Feb 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should output a Sequence. The DUR variable is not set using Delay objects, we may need to change that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea now is to add a duration to the ouput Sequence

Copy link
Member

@cncastillo cncastillo Mar 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seq.DUR needs to be a vector of Delay's. To be able to combine them as we talked #4

dly = Delay(rf.delay + rf.T + ring_down_time) # I NEED a review
end

return rf, gz, gzr, dly
Copy link
Member

@cncastillo cncastillo Feb 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should output a Sequence. The Delay should be the DUR.

dly = Delay(rf.delay + rf.T + ring_down_time) # I NEED a review
end

return rf, gz, gzr, delay
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should output a Sequence

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests should verify that the gradient/rf/adc satisfies the design inputs (area of the gradiet, duration, etc)

Comment on lines +6 to +9
if !isnothing(sys)
dead_time = sys.ADC_dead_time_T
Δtadc = sys.ADC_Δt
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be required, set the default for optional parameter sys = Scanner()

Comment on lines +11 to +18
if (Δtadc == 0 && duration == 0) || (Δtadc > 0 && duration > 0)
@error "Either dwell or duration must be defined"
end
if duration > 0
Δtadc = duration / num_samles
elseif Δtadc > 0
duration = Δtadc * num_samles;
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use multiple dispatch

Comment on lines +7 to +11
if !isnothing(sys)
max_grad = sys.Gmax
max_slew = sys.Smax
Δtgr = sys.GR_Δt
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be required, set the default for optional parameter sys = Scanner()

Comment on lines +13 to +90
if area == 0 && flat_area == 0 && amplitude == 0
@error "trapezoid: invalid keywords. Must supply either 'area', 'flat_area' or 'amplitude'"
end
if fall_time > 0 && rise_time == 0
@error "trapezoid: invalid keywords. Must always supply 'rise_time' if 'fall_time' is specified explicitly."
end

if flat_time > 0
if amplitude == 0
if flat_area == 0
@error "trapezoid: invalid keyworks. When 'flat_time' is provided either 'flat_area' or 'amplitude' must be provided as well; you may consider providing 'duration', 'area' and optionally ramp times instead."
end
amplitude = flat_area / flat_time
end
if rise_time == 0
rise_time = abs(amplitude) / max_slew;
rise_time = ceil(rise_time / Δtgr) * Δtgr;
if rise_time == 0
rise_time = Δtgr
end
end
if fall_time == 0
fall_time = rise_time
end
elseif duration > 0
if amplitude == 0
if rise_time == 0
dC = 1 / abs(2 * max_slew) + 1 / abs(2 * max_slew)
possible = duration^2 > 4 * abs(area) * dC;
@assert possible "Requested area is too large for this gradient. Minimum required duration (assuming triangle gradient can be realized) is $(round(sqrt(4 * abs(area) * dC) * 1e6)) us"
amplitude = (duration - sqrt(duration^2 - 4 * abs(area) * dC)) / (2 * dC)
else
if fall_time == 0
fall_time = rise_time
end
amplitude = area / (duration - 0.5 * rise_time - 0.5 * fall_time)
possible = duration > (rise_time + fall_time) && abs(amplitude) < max_grad
@assert possible "Requested area is too large for this gradient. Probably amplitude is violated ($(round(abs(amplitude) / max_grad * 100))%)"
end
end
if rise_time == 0
rise_time = ceil(abs(amplitude) / max_slew / Δtgr) * Δtgr
if rise_time == 0
rise_time = Δtgr
end
end
if fall_time == 0
fall_time = rise_time
end
flat_time = duration - rise_time - fall_time
if amplitude == 0
# Adjust amplitude (after rounding) to achieve given area
amplitude = area / (rise_time / 2 + fall_time / 2 + flat_time)
end
else
if area == 0
@error "trapezoid: invalid keywords. Must supply area or duration"
else
# find the shortest possible duration
# first check if the area can be realized as a triangle
# if not we calculate a trapezoid
rise_time = ceil(sqrt(abs(area) / max_slew) / Δtgr) * Δtgr
if rise_time < Δtgr # the "area" was probably 0 or almost 0 ...
rise_time = Δtgr;
end
amplitude = area / rise_time
t_eff = rise_time
if abs(amplitude) > max_grad
t_eff = ceil(abs(area) / max_grad / Δtgr) * Δtgr
amplitude = area / t_eff
if rise_time == 0
rise_time = Δtgr
end
end
flat_time = t_eff - rise_time
fall_time = rise_time
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use multiple dispatch to simplify this, the code is too convoluted

end
end

@assert abs(amplitude) <= max_grad "trapezoid: invalid amplitude. Amplitude violation ($(round(abs(amplitude) / max_grad * 100))%)"
Copy link
Member

@cncastillo cncastillo Feb 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use functions to check hardware limits for the input sys.

Comment on lines +102 to +106
if !isnothing(sys)
max_grad = sys.Gmax
max_slew = sys.Smax
Δtgr = sys.GR_Δt
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not required

Comment on lines +108 to +112
slew = (waveform[2:end] - waveform[1:end-1]) / Δtgr
if !isempty(slew)
@assert maximum(abs.(slew)) <= max_slew "Slew rate violation ($(maximum(abs.(slew)) / max_slew * 100)%)"
end
@assert maximum(abs.(waveform)) <= max_grad "Gradient amplitude violation ($(maximum(abs.(waveform)) / max_grad * 100)%)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use functions to check this.

Comment on lines 12 to 20
if duration == 0
if time_bw_product > 0
duration = time_bw_product / bandwidth
elseif bandwidth > 0
duration = 1 / (4 * bandwidth)
else
@error "Either bandwidth or duration must be defined"
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mulitiple dispatch

Comment on lines 7 to 10
if !isnothing(sys)
dead_time = sys.RF_dead_time_T
ring_down_time = sys.RF_ring_down_T
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not required

Copy link

codecov bot commented Feb 2, 2024

Codecov Report

Attention: 57 lines in your changes are missing coverage. Please review.

Comparison is base (6469a10) 92.60% compared to head (133a7b1) 90.77%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #305      +/-   ##
==========================================
- Coverage   92.60%   90.77%   -1.84%     
==========================================
  Files          33       36       +3     
  Lines        2245     2417     +172     
==========================================
+ Hits         2079     2194     +115     
- Misses        166      223      +57     
Flag Coverage Δ
base 88.19% <66.66%> (-4.49%) ⬇️
core 90.47% <ø> (ø)
files 92.34% <ø> (ø)
komamri 93.89% <ø> (ø)
plots 93.28% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
KomaMRIBase/src/sequences/PulseDesigner.jl 94.11% <ø> (+0.05%) ⬆️
KomaMRIBase/src/sequences/ADC/design.jl 57.14% <57.14%> (ø)
KomaMRIBase/src/sequences/Grad/design.jl 63.76% <63.76%> (ø)
KomaMRIBase/src/sequences/RF/design.jl 70.45% <70.45%> (ø)

Comment on lines 23 to 25
if dead_time > delay
delay = dead_time
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delay = max(dead_time, delay)

Comment on lines +41 to +46
if !isnothing(sys)
dead_time = sys.RF_dead_time_T
ring_down_time = sys.RF_ring_down_T
Δtrf = sys.RF_Δt
Δtgr = sys.GR_Δt
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not required.

Copy link
Member

@cncastillo cncastillo Feb 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate functions in different files. All of them, not only the ones in ADC.

if dead_time > delay
delay = dead_time
end
rf = RF(signal, duration, freq_offset, delay)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation problem.

Comment on lines +62 to +64
if dead_time > delay
delay = dead_time
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delay = max(delay, dead_time)

amplitude = BW / slice_thickness
area = amplitude * duration
gz = trapezoid(; flat_time=duration, flat_area=area, sys=sys);
gz_area = gz.A * (gz.T + gz.rise / 2 + gz.fall / 2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need a function to calculate the area of a Grad (get_kspace almost does this)


N = length(signal)
duration = (N-1) * Δtrf
t = range(0, duration; length=N)
Copy link
Member

@cncastillo cncastillo Feb 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this even being used? ( t = range(0, duration; length=N))

Comment on lines +142 to +144
if rf.delay < gz.rise + gz.delay
rf.delay = gz.rise + gz.delay # these are on the grad raster already which is coarser
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delay = max(...)

Comment on lines +146 to +149
dly = Delay(0)
if ring_down_time > 0
dly = Delay(rf.delay + rf.T + ring_down_time) # I NEED a review
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ring_down_time is not > 0, it must be zero, therefore dly = Delay(rf.delay + rf.T + ring_down_time) is always correct

t = range(0, duration; length=N)
window = (1 - apodization) .+ apodization * cos.(2π * ((t .- (centerpos * duration)) / duration))
signal = window .* sinc.(BW * (t .- (centerpos * duration)))
flip = 0.5 * sum(signal[2:end] + signal[1:end-1]) * Δtrf * 2π
Copy link
Member

@cncastillo cncastillo Feb 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use get_flip_angle function, I think this assumes that signal is in a particular unit. Is missing the multiplication by gamma.

@cncastillo cncastillo mentioned this pull request Mar 13, 2024
14 tasks
Comment on lines 6 to 35
#Angle{T} = Union{Quantity{T, NoDims, typeof(u"rad")}, Quantity{T, NoDims, typeof(u"°")}} where T

function KomaMRIBase.block_pulse(x::Real)
return "Hola"
end

#function KomaMRIBase.block_pulse(duration::Unitful.Time;
# phase_offset::Angle=0u"°", freq_offset::Frequency=0u"Hz", delay=0u"s", sys=Scanner())
# duration, phase_offset, freq_offset, delay = upreferred.((duration, phase_offset, freq_offset, delay))
# return PulseDesigner.block_pulse(FlipAngle(π/2), Duration(duration); phase_offset, freq_offset, delay, sys)
#end


#function PulseDesigner.block_pulse(flip_angle::Angle, duration::Unitful.Time;
# phase_offset::Angle=0u"°", freq_offset::Frequency=0u"Hz", delay=0u"s", sys=Scanner())
# flip_angle, duration, phase_offset, freq_offset, delay = upreferred.((flip_angle, duration, phase_offset, freq_offset, delay))
# return PulseDesigner.block_pulse(FlipAngle(flip_angle), Duration(duration); phase_offset, freq_offset, delay, sys)
#end
#
#function PulseDesigner.block_pulse(flip_angle::Angle, bandwidth::Unitful.Frequency;
# phase_offset::Angle=0u"°", freq_offset::Frequency=0u"Hz", delay=0u"s", sys=Scanner())
# flip_angle, bandwidth, phase_offset, freq_offset, delay = upreferred.((flip_angle, bandwidth, phase_offset, freq_offset, delay))
# return PulseDesigner.block_pulse(FlipAngle(flip_angle), Bandwidth(bandwidth); phase_offset, freq_offset, delay, sys)
#end
#
#function PulseDesigner.block_pulse(flip_angle::Angle, bandwidth::Unitful.Frequency, time_bw_product::DimensionlessQuantity;
# phase_offset::Angle=0u"°", freq_offset::Frequency=0u"Hz", delay=0u"s", sys=Scanner())
# flip_angle, bandwidth, time_bw_product, phase_offset, freq_offset, delay = upreferred.((flip_angle, bandwidth, time_bw_product, phase_offset, freq_offset, delay))
# return PulseDesigner.block_pulse(FlipAngle(flip_angle), Bandwidth(bandwidth), TimeBwProduct(time_bw_product); phase_offset, freq_offset, delay, sys)
#end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??

struct Duration val end
struct Bandwidth val end
struct TimeBwProduct val end
struct Angle end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
struct Angle end
struct Angle val end

Comment on lines 7 to 8
dead_time = sys.RF_dead_time_T
ring_down_time = sys.RF_ring_down_T
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why define these?

block_duration = delay + duration + ring_down_time

rf = RF(amplitude, duration, freq_offset, delay)
return Sequence([Grad(0, 0);;], [rf;;], [ADC(0, 0)], block_duration)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need a way to combine events so this is easier, seq += [rf, block_duration] where rf::RF and block_duration::Delay, issue #4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants