Closed aTrotier closed 2 years ago
Merging #79 (eb6e54f) into master (efb3ac0) will increase coverage by
0.20%
. The diff coverage is0.00%
.:exclamation: Current head eb6e54f differs from pull request most recent head dadc68d. Consider uploading reports for the commit dadc68d to get more accurate results
@@ Coverage Diff @@
## master #79 +/- ##
==========================================
+ Coverage 63.87% 64.07% +0.20%
==========================================
Files 37 37
Lines 1550 1545 -5
==========================================
Hits 990 990
+ Misses 560 555 -5
Impacted Files | Coverage Δ | |
---|---|---|
src/Operators/Composition.jl | 80.00% <ø> (ø) |
|
src/Operators/EncodingOp.jl | 50.84% <0.00%> (ø) |
|
src/Operators/ExplicitOp.jl | 92.30% <ø> (ø) |
|
src/Operators/FieldmapNFFTOp.jl | 87.85% <ø> (ø) |
|
src/Operators/NFFTOp.jl | 79.24% <ø> (ø) |
|
src/Sampling/Sampling.jl | 81.08% <ø> (+9.65%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update efb3ac0...dadc68d. Read the comment docs.
Adding a circshift along the slice direction i get a rmse under 10 percent :
b = BrukerFile("data/UTE3D_NR2/")
raw = RawAcquisitionData_3DUTE(b)
acq = AcquisitionData(raw); # TODO vérification des modifications qui ont étaient effectuée
params = Dict{Symbol, Any}()
params[:reco] = "direct"
if (acq.encodingSize[3]>1)
params[:reconSize] = (acq.encodingSize[1],acq.encodingSize[2],acq.encodingSize[3]);
else
params[:reconSize] = (acq.encodingSize[1],acq.encodingSize[2]);
end
Ireco = reconstruction(acq, params);
#@test size(Ireco) == (raw.params["encodedSize"][1], raw.params["encodedSize"][2], raw.params["encodedSize"][3], 1, raw.params["receiverChannels"])
Isos = sqrt.(sum(abs.(Ireco).^2,dims=5));
Isos = Isos ./ maximum(Isos);
I2dseq = recoData(b)
I2dseq = I2dseq ./ maximum(I2dseq);
# reorient
I2dseq = permutedims(I2dseq,(2,1,3,4))
I2dseq = circshift(I2dseq,(0,0,1,0))
#@test norm(vec(I2dseq)-vec(Isos))/norm(vec(I2dseq)) < listNormValues[i]
sl = 60
p1 = Plots.heatmap(abs.(Isos[:,:,sl,1,1]),title = "MRIReco",color = :greys,aspect_ratio = 1, axis=nothing, lims=(1,95))
p2 = Plots.heatmap(abs.(I2dseq[:,:,sl,1]),title = "Bruker",color = :greys,aspect_ratio = 1 , axis=nothing, lims=(1,95))
diff = abs.(abs.(I2dseq[:,:,sl,1]) - abs.(Isos[:,:,sl,1,1] )) ./ abs.(I2dseq[:,:,20,1])
p3 = Plots.heatmap(diff,title = "Diff",color = :greys,aspect_ratio = 1 , axis=nothing, lims=(1,95))
Plots.plot(p1,p2,p3,layouts=(1,3))
MRIReco.norm(vec(I2dseq)-vec(Isos))/MRIReco.norm(vec(I2dseq))
Great that we have support for more sequences. What I don't get is, why we need trajectoryCustom
, rawdataCustom
and RawAcquisitionData_3DUTE
. Its fine that we have custom code but the dispatch should be done within the functions RawAcquisitionData
, trajectory
and rawdata
.
It was just a first try to clearly separate the functions in order to discuss the implementation ?
If you are ok with it I can regroup that under RawAcquisitionData, trajectory and rawdata.
Can you upload the dataset in order to create a test ?
Can you upload the dataset in order to create a test ?
Yes, will do later this week.
Thanks
The data is now included, see https://github.com/MagneticResonanceImaging/MRIReco.jl/commit/eb6e54f67732c1247b3f1be6d64db0a602bdcd8b
It is in the folder BrukerFile/3D_UTE_NR2/
To test this locally, you need to dev MRIFiles/MRIBase as well.
Thanks,
with dev MRIFiles/MRIBase
my test is working.
Should I change the version of MRIFiles/MRIBase in order to make the CI tests works ?
No, I will do this. Are you already using the artifact?
yes. I need to dev MRIFiles and MRIBase. The CI is not using the version in that PR. Am I missing something to make it work without making a PR of first MRIBase/MRIFiles ?
Am I missing something to make it work without making a PR of first MRIBase/MRIFiles ?
no. There is no chance that you can make CI happy in this setting. I will try out locally and then do the releases.
I made the releases but CI is only partially happy. It seems that there is probably just not enough memory for the UTE dataset on the windows CI: https://github.com/MagneticResonanceImaging/MRIReco.jl/runs/6240907107?check_suite_focus=true
Ok I will acquire a smaller dataset after ismrm
Le sam. 30 avr. 2022 à 18:06, Tobias Knopp @.***> a écrit :
I made the releases but CI is only partially happy. It seems that there is probably just not enough memory for the UTE dataset on the windows CI: https://github.com/MagneticResonanceImaging/MRIReco.jl/runs/6240907107?check_suite_focus=true
— Reply to this email directly, view it on GitHub https://github.com/MagneticResonanceImaging/MRIReco.jl/pull/79#issuecomment-1114012125, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC5P7O2AO56DHKAFCOG4STDVHVK67ANCNFSM5TK5KXVA . You are receiving this because you authored the thread.Message ID: @.***>
This PR is not suppose to be merged for now
Goal
It is an initial implementation of a 3DUTE reconstruction from the standard 3D UTE Bruker acquisition.
Test
You can find a testing script here with the associated datasets (a 3DUTE with a matrix of 96x96x96 and 2 NR) https://drive.google.com/drive/folders/1roh0tRhAQTsqe27iKaHHhuGGi9n4MA65?usp=sharing
Results of test.jl :
RMSE = 0.30
Remarks
function RawAcquisitionDataFid(b::BrukerFile)
. We should discuss how to manage the factorization.encStep
problem by creating custom functions only called if :if (f.params["trajectory"] == "custom")
Again a lot of repetition with the standard functions but I am not using the EncStep1/EncStep2 (which are all equal to 0 during the conversion from bruker to acq).Discussions
1- Reconstruction only return 1 repetition (the for loop is not available for NR) 2- Only one trajectory is stored during conversion from raw to acq, should we keep one for each NR,contrast,slices ?
Let me know what are your thoughts about that first try.