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

3D recon #324

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

3D recon #324

wants to merge 55 commits into from

Conversation

curtcorum
Copy link
Contributor

@curtcorum curtcorum commented Mar 13, 2024

Draft pull request for enabling 3D reconstruction.

See discussion: Reconstruction for 3D sequences #323

modified:   ../KomaMRICore/src/rawdata/ISMRMRD.jl
modified:   KomaUI.jl
@curtcorum curtcorum marked this pull request as draft March 13, 2024 03:26
@curtcorum
Copy link
Contributor Author

curtcorum commented Mar 13, 2024

Managed to break 2D...looks like separate handling of 2D vs 3D is required in each function.

Probably best to do non_ui funnctions first, test, then ui functions.

shell> git branch -v
* 3D_recon          674c0f2 Line 123 setup_raw() needs modifications for 3D? modified:   ExportUIFunctions.jl
  ccdev             f0bfc86 Working on HSn....lots of problems. modified:   KomaMRIBase/src/sequences/PulseDesigner.jl
  master            0a8a93d Update README.md
  phantom_info      a86b7bb Merge branch 'JuliaHealth:master' into upsample_phantoms
  upsample_phantoms a86b7bb Merge branch 'JuliaHealth:master' into upsample_phantoms

Progress: 100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| Time: 0:00:44
  simulated_blocks:  22
  rf_blocks:         1
  acq_samples:       10201
 44.835454 seconds (43.57 M allocations: 2.874 GiB, 2.96% gc time, 86.70% compilation time: <1% of which was recompilation)
[ Info: Exporting to ISMRMRD file: /tmp/Koma_signal.mrd
[ Info: Running reconstruction ...
ArgumentError: Nodes x have dimension 2 != 3
Stacktrace:
  [1] initParams(k::Matrix{Float64}, N::Tuple{Int64, Int64, Int64}, dims::UnitRange{Int64}; kargs::Base.Pairs{Symbol, Int64, Tuple{Symbol, Symbol}, NamedTuple{(:m, :σ), Tuple{Int64, Int64}}})
    @ NFFT ~/.julia/packages/NFFT/RT2hs/src/precomputation.jl:20
  [2] initParams
    @ ~/.julia/packages/NFFT/RT2hs/src/precomputation.jl:3 [inlined]
  [3] NFFT.NFFTPlan(k::Matrix{Float64}, N::Tuple{Int64, Int64, Int64}; dims::UnitRange{Int64}, fftflags::Nothing, kwargs::Base.Pairs{Symbol, Int64, Tuple{Symbol, Symbol}, NamedTuple{(:m, :σ), Tuple{Int64, Int64}}})
    @ NFFT ~/.julia/packages/NFFT/RT2hs/src/implementation.jl:78
  [4] NFFTPlan
    @ ~/.julia/packages/NFFT/RT2hs/src/implementation.jl:73 [inlined]
  [5] macro expansion
    @ ~/.julia/packages/NFFT/RT2hs/src/NFFT.jl:49 [inlined]
  [6] macro expansion
    @ ./timing.jl:393 [inlined]
  [7] #plan_nfft#130
    @ ~/.julia/packages/NFFT/RT2hs/src/NFFT.jl:48 [inlined]
  [8] plan_nfft
    @ ~/.julia/packages/NFFT/RT2hs/src/NFFT.jl:46 [inlined]
  [9] #plan_nfft#2
    @ ~/.julia/packages/AbstractNFFTs/Xd3qS/src/derived.jl:13 [inlined]
 [10] plan_nfft
    @ ~/.julia/packages/AbstractNFFTs/Xd3qS/src/derived.jl:13 [inlined]
 [11] samplingDensity(acqData::AcquisitionData{Float32, 2}, shape::Tuple{Int64, Int64, Int64})
    @ MRIBase ~/.julia/packages/MRIBase/oNfYy/src/Datatypes/AcqData.jl:288
 [12] setupDirectReco(acqData::AcquisitionData{Float32, 2}, recoParams::Dict{Symbol, Any})
    @ MRIReco ~/src/MRIReco.jl/src/Reconstruction/RecoParameters.jl:19
 [13] reconstruction(acqData::AcquisitionData{Float32, 2}, recoParams::Dict{Symbol, Any})
    @ MRIReco ~/src/MRIReco.jl/src/Reconstruction/Reconstruction.jl:37
 [14] macro expansion
    @ ./timing.jl:501 [inlined]
 [15] (::KomaMRI.var"#61#97"{Dict{Symbol, Any}, Blink.AtomShell.Window})(#unused#::Int64)
    @ KomaMRI ~/src/KomaMRI/src/KomaUI.jl:230
 [16] #invokelatest#2
    @ ./essentials.jl:819 [inlined]
 [17] invokelatest
    @ ./essentials.jl:816 [inlined]
 [18] handle_message(o::Blink.Page, m::Dict{String, Any})
    @ Blink ~/.julia/packages/Blink/tnO3a/src/rpc/callbacks.jl:7
 [19] macro expansion
    @ ~/.julia/packages/Lazy/9Xnd3/src/macros.jl:268 [inlined]
 [20] ws_handler(ws::HTTP.WebSockets.WebSocket)
    @ Blink ~/.julia/packages/Blink/tnO3a/src/content/server.jl:50
 [21] (::Mux.var"#9#10"{Mux.App})(sock::HTTP.WebSockets.WebSocket)
    @ Mux ~/.julia/packages/Mux/PipQ9/src/server.jl:48
 [22] upgrade(f::Mux.var"#9#10"{Mux.App}, http::HTTP.Streams.Stream{HTTP.Messages.Request, HTTP.Connections.Connection{Sockets.TCPSocket}}; suppress_close_error::Bool, maxframesize::Int64, maxfragmentation::Int64, kw::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ HTTP.WebSockets ~/.julia/packages/HTTP/1EWL3/src/WebSockets.jl:440
 [23] upgrade
    @ ~/.julia/packages/HTTP/1EWL3/src/WebSockets.jl:420 [inlined]
 [24] (::Mux.var"#14#15"{Mux.App, Mux.App})(http::HTTP.Streams.Stream{HTTP.Messages.Request, HTTP.Connections.Connection{Sockets.TCPSocket}})
    @ Mux ~/.julia/packages/Mux/PipQ9/src/server.jl:81
 [25] #invokelatest#2
    @ ./essentials.jl:819 [inlined]
 [26] invokelatest
    @ ./essentials.jl:816 [inlined]
 [27] handle_connection(f::Function, c::HTTP.Connections.Connection{Sockets.TCPSocket}, listener::HTTP.Servers.Listener{Nothing, Sockets.TCPServer}, readtimeout::Int64, access_log::Nothing)
    @ HTTP.Servers ~/.julia/packages/HTTP/1EWL3/src/Servers.jl:456
 [28] macro expansion
    @ ~/.julia/packages/HTTP/1EWL3/src/Servers.jl:388 [inlined]
 [29] (::HTTP.Servers.var"#16#17"{Mux.var"#14#15"{Mux.App, Mux.App}, HTTP.Servers.Listener{Nothing, Sockets.TCPServer}, Set{HTTP.Connections.Connection}, Int64, Nothing, Base.Semaphore, HTTP.Connections.Connection{Sockets.TCPSocket}})()
    @ HTTP.Servers ./task.jl:514

@cncastillo
Copy link
Member

Thanks @curtcorum for all the effort!! This is similar to what I would have tried. I will try to take a further look asap.

src/KomaUI.jl Outdated
@@ -221,8 +221,8 @@ function KomaUI(; darkmode=true, frame=true, phantom_mode="2D", sim=Dict{String,
acq_data = AcquisitionData(raw_aux)
acq_data.traj[1].circular = false #Removing circular window
acq_data.traj[1].nodes = acq_data.traj[1].nodes[1:2,:] ./ maximum(2*abs.(acq_data.traj[1].nodes[:])) #Normalize k-space to -.5 to .5 for NUFFT
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this line is generating problems.

Suggested change
acq_data.traj[1].nodes = acq_data.traj[1].nodes[1:2,:] ./ maximum(2*abs.(acq_data.traj[1].nodes[:])) #Normalize k-space to -.5 to .5 for NUFFT
acq_data.traj[1].nodes = acq_data.traj[1].nodes[1:3,:] ./ maximum(2*abs.(acq_data.traj[1].nodes[:])) #Normalize k-space to -.5 to .5 for NUFFT

Nx = round(Int64, FOVx / Δx[1])
Ny = round(Int64, FOVy / Δx[2])
Nz = round(Int64, FOVy / Δx[3])
Copy link
Member

Choose a reason for hiding this comment

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

The Ci gives an error saying that NadcsPerImage = floor(Int, Nadcs / Nz) => Nz = 0. This line could be the problem.

Suggested change
Nz = round(Int64, FOVy / Δx[3])
Nz = round(Int64, FOVz / Δx[3])

I wonder if Δx[3] is Inf.

@curtcorum
Copy link
Contributor Author

@cncastillo

Thanks @curtcorum for all the effort!! This is similar to what I would have tried. I will try to take a further look asap.

Thanks and my pleasure, Carlos!

I'm very interested in doing 3D simulation, especially of 3D Radial sequences, anything I can do to help.

I certainly greatly appreciate your, @beorostica, and the other developers and users work on KomaMRI!

@curtcorum
Copy link
Contributor Author

curtcorum commented Mar 20, 2024

Some comments/notes on 3D.

The UI and plots get bogged down with large seq and raw files.

It is potentially desirable for the UI to parameterize the maximum range of sequence blocks (or time range) for the seq slider. Then one could have a second slider, or stop rendering the slider and just show the range displayed in the main seq window. Not sure how to do this but something to think about.

Also there are similar UI or plot slowing down issues when the 2D or 3D phantom has a lot of points.

For the UI it might make sense to have some default and changeable parameters passed to KomaUI() in the manner of sim= and rec=:

ui_params = Dict{String,Any}()

'max_blocks', 500 # these are guesses for now

'max_raw', 2000

'max_points', 25000 # could sub-sample after some threshold

...

@cncastillo
Copy link
Member

The UI and plots get bogged down with large seq and raw files.

You are absolutly right.  I would just add ⬅️ ➡️ buttons and always render a max of 100 blocks maybe? The same could be done for plot_raw. @pvillacorta has played with something similar in #184 (f19a750).

I was imagining something like this

216764329-94b9cd7e-fee9-439b-9134-95b7be626592

https://community.home-assistant.io/t/plotly-interactive-graph-card/347746

Not sure if this solution will fix the problem.

@cncastillo cncastillo linked an issue May 17, 2024 that may be closed by this pull request
modified:   KomaMRIBase/src/datatypes/Sequence.jl
modified:   KomaMRIBase/src/datatypes/Sequence.jl
modified:   KomaMRICore/src/KomaMRICore.jl
modified:   KomaMRICore/src/rawdata/ISMRMRD.jl
…counter?

modified:   KomaMRICore/src/rawdata/ISMRMRD.jl
…RIReco. Need to configure reco_parameters.

modified:   KomaMRICore/src/rawdata/ISMRMRD.jl
More updates for FOV estimation
One more debug?
modified:   KomaMRICore/src/rawdata/ISMRMRD.jl
modified:   KomaMRICore/src/rawdata/ISMRMRD.jl
@curtcorum
Copy link
Contributor Author

curtcorum commented Jun 12, 2024

Local tests* of KomaMRICore went ok on Ubuntu 20.04

Linux green 5.15.0-107-generic #117~20.04.1-Ubuntu SMP Tue Apr 30 10:35:57 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Precompiling project...
  ✗ Metal → BFloat16sExt
  201 dependencies successfully precompiled in 97 seconds. 9 already precompiled.
  2 dependencies had output during precompilation:
┌ AbstractNFFTs
│  WARNING: method definition for #plan_nfft#3 at /home/curt/.julia/packages/AbstractNFFTs/Xd3qS/src/derived.jl:16 declares type variable D but does not use it.
│  WARNING: method definition for #plan_nfft#4 at /home/curt/.julia/packages/AbstractNFFTs/Xd3qS/src/derived.jl:21 declares type variable D but does not use it.
│  WARNING: method definition for #plan_nfct#8 at /home/curt/.julia/packages/AbstractNFFTs/Xd3qS/src/derived.jl:16 declares type variable D but does not use it.
│  WARNING: method definition for #plan_nfct#9 at /home/curt/.julia/packages/AbstractNFFTs/Xd3qS/src/derived.jl:21 declares type variable D but does not use it.
│  WARNING: method definition for #plan_nfst#13 at /home/curt/.julia/packages/AbstractNFFTs/Xd3qS/src/derived.jl:16 declares type variable D but does not use it.
│  WARNING: method definition for #plan_nfst#14 at /home/curt/.julia/packages/AbstractNFFTs/Xd3qS/src/derived.jl:21 declares type variable D but does not use it.
│  WARNING: method definition for #plan_nnfft#17 at /home/curt/.julia/packages/AbstractNFFTs/Xd3qS/src/derived.jl:34 declares type variable D but does not use it.
└  
┌ AMDGPU
│  WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
│  You may need to write `f(x::Vararg{T})` rather than `f(x::Vararg{<:T})` or `f(x::Vararg{T}) where T` instead of `f(x::Vararg{T} where T)`.
│  WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
│  You may need to write `f(x::Vararg{T})` rather than `f(x::Vararg{<:T})` or `f(x::Vararg{T}) where T` instead of `f(x::Vararg{T} where T)`.
└  
     Testing Running tests...
┌ Warning: Estimating FOV parameters from seq.DEF Nx, Ny, Nz and traj.
└ @ KomaMRICore ~/src/KomaMRI/KomaMRICore/src/rawdata/ISMRMRD.jl:301
┌ Debug: Nd_seq = 2
└ @ KomaMRICore ~/src/KomaMRI/KomaMRICore/src/rawdata/ISMRMRD.jl:308
┌ Debug: Nx, Ny, Nz, Ns = 101, 101, 1, 1
└ @ KomaMRICore ~/src/KomaMRI/KomaMRICore/src/rawdata/ISMRMRD.jl:309
┌ Debug: FOVx, FOVy, FOVz = 0.23229999999419157, 0.2323000000111597, 1
└ @ KomaMRICore ~/src/KomaMRI/KomaMRICore/src/rawdata/ISMRMRD.jl:310
┌ Debug: Δx, size( ktraj) = [0.002299999999942491, 0.002300000000110492, 1.0e-6], (10201, 3)
└ @ KomaMRICore ~/src/KomaMRI/KomaMRICore/src/rawdata/ISMRMRD.jl:311
[ Info: Creating 2D ISMRMRD file ...
┌ Debug: Nd_seq = 2
└ @ KomaMRICore ~/src/KomaMRI/KomaMRICore/src/rawdata/ISMRMRD.jl:308
┌ Debug: Nx, Ny, Nz, Ns = 101, 101, 0, 1
└ @ KomaMRICore ~/src/KomaMRI/KomaMRICore/src/rawdata/ISMRMRD.jl:309
┌ Debug: FOVx, FOVy, FOVz = 0.23, 0.23, 0.0
└ @ KomaMRICore ~/src/KomaMRI/KomaMRICore/src/rawdata/ISMRMRD.jl:310
┌ Debug: Δx, size( ktraj) = [0.002299999999942491, 0.002300000000110492, 1.0e-6], (10201, 3)
└ @ KomaMRICore ~/src/KomaMRI/KomaMRICore/src/rawdata/ISMRMRD.jl:311
[ Info: Creating 2D ISMRMRD file ...
Test Summary: | Pass  Total     Time
Package       |   81     81  2m17.6s

modified:   KomaMRICore/src/rawdata/ISMRMRD.jl
@curtcorum
Copy link
Contributor Author

Probably the checks on the recon parameters should be moved to one place?

#409 (comment)

@cncastillo
Copy link
Member

cncastillo commented Jun 19, 2024

Yeah it should be its own function. Currently we are finishing of #411 and #408, and I can go back to the sequences and this.

…ata/ISMRMRD.jl

modified:   KomaMRICore/src/KomaMRICore.jl
modified:   KomaMRICore/src/rawdata/ISMRMRD.jl
modified:   KomaMRIFiles/src/Sequence/Pulseq.jl
…ot for spiral

modified:   KomaMRICore/src/rawdata/ISMRMRD.jl
Conflicts: (fixed)
KomaMRICore/src/rawdata/ISMRMRD.jl

Changes to be committed:
modified:   KomaMRIBase/Project.toml
modified:   KomaMRIBase/src/KomaMRIBase.jl
modified:   KomaMRICore/Project.toml
modified:   KomaMRICore/src/KomaMRICore.jl
modified:   KomaMRICore/src/rawdata/ISMRMRD.jl
modified:   KomaMRICore/src/simulation/SimulatorCore.jl
modified:   KomaMRICore/test/Project.toml
deleted:    KomaMRICore/test/initialize.jl
new file:   KomaMRICore/test/initialize_backend.jl
modified:   KomaMRICore/test/runtests.jl
modified:   KomaMRIFiles/Project.toml
modified:   KomaMRIFiles/src/KomaMRIFiles.jl
modified:   KomaMRIFiles/src/Phantom/Phantom.jl
modified:   KomaMRIPlots/Project.toml
modified:   KomaMRIPlots/src/KomaMRIPlots.jl
modified:   Project.toml
modified:   src/KomaMRI.jl
modified:   src/KomaUI.jl
modified:   KomaMRICore/src/rawdata/ISMRMRD.jl
modified:   KomaMRICore/src/rawdata/ISMRMRD.jl
Was not matching ::Tuple{Vararg{Int64, D}} in MRIBase ~/.julia/packages/MRIBase/GskMD/src/Datatypes/AcqData.jl:21
modified:   KomaMRICore/src/rawdata/ISMRMRD.jl
Need to have Nx, etc. be sequence structure and send separate matrix size for recon parameters.
modified:   Project.toml
modified:   src/KomaMRICore.jl
modified:   src/rawdata/ISMRMRD.jl
modified:   ../Project.toml
Conflicts:
	KomaMRICore/src/simulation/GPUFunctions.jl
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.

Reconstruction for 3D sequences
2 participants