-
Notifications
You must be signed in to change notification settings - Fork 37
Parallel #113
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
Parallel #113
Changes from all commits
faea568
a0ad54a
bc69159
3e446fe
ee96f11
0c1067e
cace5b1
23e711c
bdcd9b9
076b776
fdc74db
1e3cecd
d64fd6e
2f4174c
aeee141
270b95d
ae2bd26
127d966
1c6d6f2
04a19d3
1e9e2c6
bd265c4
2c10cbd
0b7af8f
51bd03a
a75d9c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
__pycache__/ | ||
*.nii.gz | ||
*.nii | ||
*.npy | ||
*.dcm | ||
*.npy | ||
*.mat | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,28 +5,33 @@ | |
import argparse | ||
import os | ||
from utilities.data_simulation.Download_data import download_data | ||
import pathlib | ||
|
||
########## | ||
# code written by Oliver J Gurney-Champion | ||
# code adapted from MAtlab code by Eric Schrauben: https://github.com/schrau24/XCAT-ERIC | ||
# This code generates a 4D IVIM phantom as nifti file | ||
|
||
def phantom(bvalue, noise, TR=3000, TE=40, motion=False, rician=False, interleaved=False,T1T2=True): | ||
download_data() | ||
np.random.seed(42) | ||
if motion: | ||
states = range(1,21) | ||
else: | ||
states = [1] | ||
for state in states: | ||
# Load the .mat file | ||
mat_data = loadmat('../../download/Phantoms/XCAT_MAT_RESP/XCAT5D_RP_' + str(state) + '_CP_1.mat') | ||
project_root = pathlib.Path(__file__).resolve().parent.parent | ||
filename = f'XCAT5D_RP_{state}_CP_1.mat' | ||
mat_path = project_root / '..' /'download' / 'Phantoms' / 'XCAT_MAT_RESP' / filename | ||
mat_data = loadmat(mat_path) | ||
|
||
# Access the variables in the loaded .mat file | ||
XCAT = mat_data['IMG'] | ||
XCAT = XCAT[-1:0:-2,-1:0:-2,10:160:4] | ||
|
||
D, f, Ds = contrast_curve_calc() | ||
S, Dim, fim, Dpim, legend = XCAT_to_MR_DCE(XCAT, TR, TE, bvalue, D, f, Ds,T1T2=T1T2) | ||
S, Dim, fim, Dpim, legend = XCAT_to_MR_IVIM(XCAT, TR, TE, bvalue, D, f, Ds,T1T2=T1T2) | ||
if state == 1: | ||
Dim_out = Dim | ||
fim_out = fim | ||
|
@@ -187,7 +192,7 @@ def contrast_curve_calc(): | |
return D, f, Ds | ||
|
||
|
||
def XCAT_to_MR_DCE(XCAT, TR, TE, bvalue, D, f, Ds, b0=3, ivim_cont = True, T1T2=True): | ||
def XCAT_to_MR_IVIM(XCAT, TR, TE, bvalue, D, f, Ds, b0=3, ivim_cont = True, T1T2=True): | ||
########################################################################################### | ||
# This script converts XCAT tissue values to MR contrast based on the SSFP signal equation. | ||
# Christopher W. Roy 2018-12-04 # [email protected] | ||
|
@@ -436,7 +441,6 @@ def parse_bvalues_file(file_path): | |
motion = args.motion | ||
interleaved = args.interleaved | ||
T1T2 = args.T1T2 | ||
download_data() | ||
for key, bvalue in bvalues.items(): | ||
bvalue = np.array(bvalue) | ||
sig, XCAT, Dim, fim, Dpim, legend = phantom(bvalue, noise, motion=motion, interleaved=interleaved,T1T2=T1T2) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import numpy as np | ||
import numpy.polynomial.polynomial as poly | ||
import warnings | ||
|
||
from utilities.data_simulation.GenerateData import GenerateData | ||
|
||
|
@@ -82,11 +83,17 @@ def ivim_fit(self, bvalues, signal): | |
# print(Dp_prime) | ||
|
||
if np.any(np.asarray(Dp_prime) < 0) or not np.all(np.isfinite(Dp_prime)): | ||
print('Perfusion fit failed') | ||
warnings.warn('Perfusion fit failed', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This applies to all the edited files in src/original. I don't think we should touch these. If there's something wrong, it should be on the authors to fix. Or we just accept it as it is and if it is flawed, then it should fail the test in question. I also don't quite get why this was needed here. If the b-value inputs are not correct, then it should be checked in the wrapper (something I have wanted to do for a while but never got around to) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I changed this as the print kept giving output into my prompt. By handling it as a warning, I can suppress the output from my testing reports. They otherwise got very long and hard to find the info I was looking for. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed that we should normally not touch them! @etpeterson, can you accept those changes to your source file? |
||
category=UserWarning, | ||
stacklevel=2 # Ensures correct file/line info in the warning | ||
) | ||
Dp_prime = [0, 0] | ||
f = signal[0] - D[0] | ||
else: | ||
print("This doesn't seem to be an IVIM set of b-values") | ||
warnings.warn('This doesn\'t seem to be an IVIM set of b-values', | ||
category=UserWarning, | ||
stacklevel=2 # Ensures correct file/line info in the warning | ||
) | ||
f = 1 | ||
Dp_prime = [0, 0] | ||
D = D[1] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,8 +114,9 @@ def ivim_fit_full_volume(self, signals, bvalues, **kwargs): | |
gtab = gradient_table(bvalues, bvec, b0_threshold=0) | ||
|
||
self.IAR_algorithm = IvimModelBiExp(gtab, bounds=self.bounds, initial_guess=self.initial_guess) | ||
|
||
fit_results = self.IAR_algorithm.fit(signals) | ||
b0_index = np.where(bvalues == 0)[0][0] | ||
mask = signals[...,b0_index]>0 | ||
fit_results = self.IAR_algorithm.fit(signals, mask=mask) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is mask a valid kwarg? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes! Otherwise the testing would fail :) |
||
|
||
results = {} | ||
results["f"] = fit_results.model_params[..., 1] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This downloads data during testing now, correct? Is that slow or burdensome?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes and no. The volume, parallel and AI tests just last long, full stop. Downloading the data is relatively quick compared to that less than minute Vs hours).
I think there are two ways we can go here.
I think I could accelerate the volume fit by dramatically reducing the number of vowels; there probably is no need for high resolution.
For the parallel fit, the smaller I make the number of vowels/volume, the less of an acceleration we get from going parallel and the harder it is to test this. For small volumes, typically linear goes way faster. That is why I ended up increasing the volume.
So either we can make a large volume (then the download time is neglectable) and do long tests. Or, we can go for a small volume and ignore the parallel timing. In that cases precalculting the volume and saving it will be faster.
That said, the AI tests also last 5+ minutes per algorithm and we must decide whether we want that to be done every merge...
Either way, all building blocks are here now :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After discussion --> this download only occurs once, so I think it is solved?