Skip to content

Implement checks for fulfilment of algorithm requirements in the OsipiBase class #96

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ nosetests.xml
coverage.xml
*.pyc
phantoms/MR_XCAT_qMRI/*.json
phantoms/MR_XCAT_qMRI/*.txt
phantoms/MR_XCAT_qMRI/*.txt

2 changes: 0 additions & 2 deletions src/standardized wip/IAR_LU_linear.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ def __init__(self, bvalues=None, thresholds=None, bounds=None, initial_guess=Non
"""
super(IAR_LU_linear, self).__init__(bvalues, thresholds, bounds, initial_guess)

# Check the inputs

# Initialize the algorithm
if self.bvalues is not None:
bvec = np.zeros((self.bvalues.size, 3))
Expand Down
6 changes: 2 additions & 4 deletions src/standardized/ETP_SRI_LinearFitting.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ def __init__(self, bvalues=None, thresholds=None, bounds=None, initial_guess=Non
Our OsipiBase object could contain functions that compare the inputs with
the requirements.
"""

super(ETP_SRI_LinearFitting, self).__init__(bvalues, thresholds, bounds, initial_guess)
if bounds is not None:
print('warning, bounds from wrapper are not (yet) used in this algorithm')
Expand All @@ -52,10 +51,8 @@ def __init__(self, bvalues=None, thresholds=None, bounds=None, initial_guess=Non
# defined with initials?
self.ETP_weighting = weighting
self.ETP_stats = stats

# Check the inputs



def ivim_fit(self, signals, bvalues=None, linear_fit_option=False, **kwargs):
"""Perform the IVIM fit

Expand All @@ -67,6 +64,7 @@ def ivim_fit(self, signals, bvalues=None, linear_fit_option=False, **kwargs):
Returns:
_type_: _description_
"""

signals[signals<0.0000001]=0.0000001
if bvalues is None:
bvalues = self.bvalues
Expand Down
5 changes: 2 additions & 3 deletions src/standardized/IAR_LU_biexp.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ def __init__(self, bvalues=None, thresholds=None, bounds=None, initial_guess=Non
print('warning, bounds from wrapper are not (yet) used in this algorithm')
self.use_bounds = False
self.use_initial_guess = False
# Check the inputs

# Initialize the algorithm
if self.bvalues is not None:
Expand All @@ -69,7 +68,7 @@ def ivim_fit(self, signals, bvalues, **kwargs):
Returns:
_type_: _description_
"""

if self.IAR_algorithm is None:
if bvalues is None:
bvalues = self.bvalues
Expand Down Expand Up @@ -101,7 +100,7 @@ def ivim_fit_full_volume(self, signals, bvalues, **kwargs):
Returns:
_type_: _description_
"""

if self.IAR_algorithm is None:
if bvalues is None:
bvalues = self.bvalues
Expand Down
3 changes: 1 addition & 2 deletions src/standardized/IAR_LU_modified_mix.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ def __init__(self, bvalues=None, thresholds=None, bounds=None, initial_guess=Non
print('warning, bounds from wrapper are not (yet) used in this algorithm')
self.use_bounds = False
self.use_initial_guess = False
# Check the inputs

# Initialize the algorithm
if self.bvalues is not None:
Expand All @@ -69,7 +68,7 @@ def ivim_fit(self, signals, bvalues, **kwargs):
Returns:
_type_: _description_
"""

if self.IAR_algorithm is None:
if bvalues is None:
bvalues = self.bvalues
Expand Down
1 change: 0 additions & 1 deletion src/standardized/IAR_LU_modified_topopro.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ def __init__(self, bvalues=None, thresholds=None, bounds=None, initial_guess=Non
print('warning, bounds from wrapper are not (yet) used in this algorithm')
self.use_bounds = False
self.use_initial_guess = False
# Check the inputs

# Initialize the algorithm
if self.bvalues is not None:
Expand Down
2 changes: 1 addition & 1 deletion src/standardized/IAR_LU_segmented_2step.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ def __init__(self, bvalues=None, thresholds=None, bounds=None, initial_guess=Non
print('warning, bounds from wrapper are not (yet) used in this algorithm')
self.use_bounds = False
self.use_initial_guess = False
# Check the inputs

# Initialize the algorithm
if self.bvalues is not None:
Expand All @@ -70,6 +69,7 @@ def ivim_fit(self, signals, bvalues, thresholds=None, **kwargs):
_type_: _description_
"""
print(thresholds)


if self.IAR_algorithm is None:
if bvalues is None:
Expand Down
1 change: 0 additions & 1 deletion src/standardized/IAR_LU_segmented_3step.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ def __init__(self, bvalues=None, thresholds=None, bounds=None, initial_guess=Non
print('warning, bounds from wrapper are not (yet) used in this algorithm')
self.use_bounds = False
self.use_initial_guess = False
# Check the inputs

# Initialize the algorithm
if self.bvalues is not None:
Expand Down
2 changes: 1 addition & 1 deletion src/standardized/IAR_LU_subtracted.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ def __init__(self, bvalues=None, thresholds=None, bounds=None, initial_guess=Non
print('warning, bounds from wrapper are not (yet) used in this algorithm')
self.use_bounds = False
self.use_initial_guess = False
# Check the inputs

# Initialize the algorithm
if self.bvalues is not None:
Expand All @@ -70,6 +69,7 @@ def ivim_fit(self, signals, bvalues, **kwargs):
_type_: _description_
"""


if self.IAR_algorithm is None:
if bvalues is None:
bvalues = self.bvalues
Expand Down
7 changes: 5 additions & 2 deletions src/standardized/OGC_AmsterdamUMC_Bayesian_biexp.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ class OGC_AmsterdamUMC_Bayesian_biexp(OsipiBase):

# Algorithm requirements
required_bvalues = 4
required_thresholds = [0,
0] # Interval from "at least" to "at most", in case submissions allow a custom number of thresholds
required_thresholds = [0,0] # Interval from "at least" to "at most", in case submissions allow a custom number of thresholds
required_bounds = False
required_bounds_optional = True # Bounds may not be required but are optional
required_initial_guess = False
Expand Down Expand Up @@ -86,6 +85,9 @@ def ivim_fit(self, signals, bvalues, initial_guess=None, **kwargs):
Returns:
_type_: _description_
"""

if initial_guess is not None and len(initial_guess) == 4:
self.initial_guess = initial_guess
bvalues=np.array(bvalues)

epsilon = 0.000001
Expand All @@ -100,5 +102,6 @@ def ivim_fit(self, signals, bvalues, initial_guess=None, **kwargs):
results["D"] = fit_results[0]
results["f"] = fit_results[1]
results["Dp"] = fit_results[2]
results["S0"] = fit_results[3]

return results
4 changes: 2 additions & 2 deletions src/standardized/OGC_AmsterdamUMC_biexp.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ class OGC_AmsterdamUMC_biexp(OsipiBase):

# Algorithm requirements
required_bvalues = 4
required_thresholds = [0,
0] # Interval from "at least" to "at most", in case submissions allow a custom number of thresholds
required_thresholds = [0,0] # Interval from "at least" to "at most", in case submissions allow a custom number of thresholds
required_bounds = False
required_bounds_optional = True # Bounds may not be required but are optional
required_initial_guess = False
Expand Down Expand Up @@ -81,5 +80,6 @@ def ivim_fit(self, signals, bvalues, **kwargs):
results["D"] = fit_results[0]
results["f"] = fit_results[1]
results["Dp"] = fit_results[2]
results["S0"] = fit_results[3]

return results
8 changes: 5 additions & 3 deletions src/standardized/OGC_AmsterdamUMC_biexp_segmented.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ class OGC_AmsterdamUMC_biexp_segmented(OsipiBase):

# Algorithm requirements
required_bvalues = 4
required_thresholds = [1,
1] # Interval from "at least" to "at most", in case submissions allow a custom number of thresholds
required_thresholds = [1,1] # Interval from "at least" to "at most", in case submissions allow a custom number of thresholds
required_bounds = False
required_bounds_optional = True # Bounds may not be required but are optional
required_initial_guess = False
Expand Down Expand Up @@ -74,13 +73,16 @@ def ivim_fit(self, signals, bvalues, **kwargs):
Returns:
_type_: _description_
"""


if self.initial_guess is not None and len(self.initial_guess) == 4:
self.initial_guess = self.initial_guess
bvalues=np.array(bvalues)
fit_results = self.OGC_algorithm(bvalues, signals, bounds=self.bounds, cutoff=self.thresholds, p0=self.initial_guess)

results = {}
results["D"] = fit_results[0]
results["f"] = fit_results[1]
results["Dp"] = fit_results[2]
results["S0"] = fit_results[3]

return results
1 change: 0 additions & 1 deletion src/standardized/OJ_GU_seg.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ def __init__(self, bvalues=None, thresholds=None, bounds=None, initial_guess=Non
print('warning, bounds from wrapper are not (yet) used in this algorithm')
self.use_bounds = False
self.use_initial_guess = False
# Check the inputs

# Initialize the algorithm

Expand Down
3 changes: 1 addition & 2 deletions src/standardized/PvH_KB_NKI_IVIMfit.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ class PvH_KB_NKI_IVIMfit(OsipiBase):

# Algorithm requirements
required_bvalues = 4
required_thresholds = [0,
0] # Interval from "at least" to "at most", in case submissions allow a custom number of thresholds
required_thresholds = [0,0] # Interval from "at least" to "at most", in case submissions allow a custom number of thresholds
required_bounds = False
required_bounds_optional = False # Bounds may not be required but are optional
required_initial_guess = False
Expand Down
78 changes: 43 additions & 35 deletions src/wrappers/OsipiBase.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,17 @@ def __init__(self, bvalues=None, thresholds=None, bounds=None, initial_guess=Non
self.initial_guess = np.asarray(initial_guess) if initial_guess is not None else None
self.use_bounds = True
self.use_initial_guess = True

# Validate the inputs
if self.bvalues is not None:
self.osipi_check_required_bvalues()
if self.thresholds is not None:
self.osipi_check_required_thresholds()
if self.bounds is not None:
self.osipi_check_required_bounds()
if self.initial_guess is not None:
self.osipi_check_required_initial_guess()

# If the user inputs an algorithm to OsipiBase, it is intereprete as initiating
# an algorithm object with that name.
if algorithm:
Expand Down Expand Up @@ -53,7 +64,8 @@ def osipi_fit(self, data, bvalues=None, **kwargs):
"""Fits the data with the bvalues
Returns [S0, f, Dstar, D]
"""

self.osipi_validate_inputs()

# We should first check whether the attributes in the __init__ are not None
# Then check if they are input here, if they are, these should overwrite the attributes
use_bvalues = bvalues if bvalues is not None else self.bvalues
Expand Down Expand Up @@ -123,6 +135,8 @@ def osipi_fit_full_volume(self, data, bvalues=None, **kwargs):
Returns:
results (dict): Dict with key each containing an array which is a parametric map.
"""

self.osipi_validate_inputs()

try:
use_bvalues = bvalues if bvalues is not None else self.bvalues
Expand Down Expand Up @@ -201,69 +215,63 @@ def osipi_print_requirements(self):
else:
print(f"Initial guess required: {self.required_initial_guess} and is not optional")

def osipi_validate_inputs(self):
"""Validates the inputs of the algorithm."""
self.osipi_check_required_bvalues()
self.osipi_check_required_thresholds()
self.osipi_check_required_bounds()
self.osipi_check_required_initial_guess()
self.osipi_accepts_dimension(self.data.ndim)

def osipi_accepted_dimensions(self):
"""The array of accepted dimensions
e.g.
(1D, 2D, 3D, 4D, 5D, 6D)
(True, True, False, False, False, False)
"""

#return (False,) * 6
return True
return getattr(self, 'accepted_dimensions', (1, 3))

def osipi_accepts_dimension(self, dim):
"""Query if the selection dimension is fittable"""

#accepted = self.accepted_dimensions()
#if dim < 0 or dim > len(accepted):
#return False
#return accepted[dim]
min_dim, max_dim = self.osipi_accepted_dimensions()
if not (min_dim <= dim <= max_dim):
raise ValueError(f"Dimension {dim}D not supported. Requires {min_dim}-{max_dim}D spatial data")
return True

def osipi_check_required_bvalues(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

b-values is another story. Here we cannot give a "default"

"""Checks if the input bvalues fulfil the algorithm requirements"""

#if self.bvalues.size < self.required_bvalues:
#print("Conformance error: Number of b-values.")
#return False
#else:
#return True
if not hasattr(self, "required_bvalues"):
raise AttributeError("required_bvalues not defined for this algorithm")
if self.bvalues is None:
raise ValueError("bvalues are not provided")
if len(self.bvalues) < self.required_bvalues:
raise ValueError(f"Atleast {self.required_bvalues} are required, but only {len(self.bvalues)} were provided")
return True

def osipi_check_required_thresholds(self):
"""Checks if the number of input thresholds fulfil the algorithm requirements"""

#if (len(self.thresholds) < self.required_thresholds[0]) or (len(self.thresholds) > self.required_thresholds[1]):
#print("Conformance error: Number of thresholds.")
#return False
#else:
#return True
if not hasattr(self, "required_thresholds"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am wondering whether for some attributes, like "required_thresholds", we should add "default" values when tests fail. Like, 200 would be an okay value for most body applications. Then we can do with a warning stating that the default value is used, instead.

raise AttributeError("required_thresholds is not defined for this algorithm")
if self.thresholds is None:
raise ValueError("thresholds are not provided")
if len(self.thresholds) < self.required_thresholds[0] and len(self.thresholds) > self.required_thresholds[1]:
raise ValueError(f"Thresholds should be between {self.required_thresholds[0]} and {self.required_thresholds[1]} but {len(self.thresholds)} were provided")
return True

def osipi_check_required_bounds(self):
"""Checks if input bounds fulfil the algorithm requirements"""
#if self.required_bounds is True and self.bounds is None:
#print("Conformance error: Bounds.")
#return False
#else:
#return True
if self.required_bounds is False and self.bounds is None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same for bounds. I think if no bounds are given, but bounds will be used, then we should give some default bounds.

raise ValueError("bounds are required but not provided")
return True

def osipi_check_required_initial_guess(self):
"""Checks if input initial guess fulfil the algorithm requirements"""

#if self.required_initial_guess is True and self.initial_guess is None:
#print("Conformance error: Initial guess")
#return False
#else:
#return True
if self.required_initial_guess is False and self.initial_guess is None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

same for initial guess :)

raise ValueError("initial_guess are required but not provided")
return True


def osipi_check_required_bvalues():
"""Minimum number of b-values required"""
pass

def osipi_author():
"""Author identification"""
return ''
Expand Down
Loading