Feature Request
Right, now there are a few problems in how simulation methods are defined.
(1) The methods are over-typed (using abstract types like KA.Backend and KomaMRICore.SiimulationMethod), meaning that is very hard to extend let's say run_spin_excitation! for a new simulation method:
function KomaMRICore.run_spin_excitation!(
p::Phantom{T},
seq::DiscreteSequence{T},
sig::AbstractArray{Complex{T}},
M::Mag{T},
sim_method::MyNewMethod, # <-----------------------------------
groupsize,
backend::KA.Backend,
pre::KomaMRICore.PreallocResult{T}
) where {T<:Real}
....
end
As you can see we need to specify too many types. Which if not done correctly gives ambiguous method errors.
(2) As the fallback is BlochSimple instead of Bloch. Extending a method, decreases the performance heavily as the undefined methods, like in the case of this example run_spin_precession!, need to be explicitly defined to use Bloch instead of BlochSimple.
(3) I don't think it is clear in the documentation how to extend simulation methods. Our what methods need to be define for a completely new simulation method. I believe prealloc, run_spin_excitation! and run_spin_precession! are enough. There are other optional methods like init_spins_state and potentially acquire_signal! (not implemented yet).
Full example showing how complicated it is now:
using KomaMRICore
import KomaMRICore.KA
struct BlochButSpoilingBeforeRF <: KomaMRICore.SimulationMethod end
function KomaMRICore.prealloc(
sim_method::BlochButSpoilingBeforeRF,
backend::KA.Backend,
obj::Phantom{T},
M::Mag{T},
max_block_length::Integer,
groupsize
) where {T<:Real}
return KomaMRICore.prealloc(KomaMRICore.Bloch(), backend, obj, M, max_block_length, groupsize)
end
function KomaMRICore.run_spin_excitation!(
p::Phantom{T},
seq::DiscreteSequence{T},
sig::AbstractArray{Complex{T}},
M::Mag{T},
sim_method::BlochButSpoilingBeforeRF,
groupsize,
backend::KA.Backend,
pre::KomaMRICore.PreallocResult{T}
) where {T<:Real}
fill!(M.xy, zero(T)) # Perfect spoiling before RF
KomaMRICore.run_spin_excitation!(p, seq, sig, M, KomaMRICore.Bloch(), groupsize, backend, pre)
return nothing
end
function KomaMRICore.run_spin_precession!(
p::Phantom{T},
seq::DiscreteSequence{T},
sig::AbstractArray{Complex{T}},
M::Mag{T},
sim_method::BlochButSpoilingBeforeRF,
groupsize,
backend::KA.Backend,
pre::KomaMRICore.PreallocResult{T}
) where {T<:Real}
KomaMRICore.run_spin_precession!(p, seq, sig, M, KomaMRICore.Bloch(), groupsize, backend, pre)
return nothing
end
and how I would like it to be:
function KomaMRICore.run_spin_excitation!(p, seq, sig, M,
sim_method::BlochButSpoilingBeforeRF, # <---------------------------- Only one required for dispatch
groupsize, backend, pre
)
fill!(M.xy, zero(eltype(M.xy))) # Perfect spoiling before RF
KomaMRICore.run_spin_excitation!(p, seq, sig, M, KomaMRICore.Bloch(), groupsize, backend, pre)
return nothing
end
Feature Request
Right, now there are a few problems in how simulation methods are defined.
(1) The methods are over-typed (using abstract types like
KA.BackendandKomaMRICore.SiimulationMethod), meaning that is very hard to extend let's sayrun_spin_excitation!for a new simulation method:As you can see we need to specify too many types. Which if not done correctly gives ambiguous method errors.
(2) As the fallback is
BlochSimpleinstead ofBloch. Extending a method, decreases the performance heavily as the undefined methods, like in the case of this examplerun_spin_precession!, need to be explicitly defined to useBlochinstead ofBlochSimple.(3) I don't think it is clear in the documentation how to extend simulation methods. Our what methods need to be define for a completely new simulation method. I believe
prealloc,run_spin_excitation!andrun_spin_precession!are enough. There are other optional methods likeinit_spins_stateand potentiallyacquire_signal!(not implemented yet).Full example showing how complicated it is now:
and how I would like it to be: