Validate derrf distribution parameters on startup #9599
Conversation
CodSpeed Performance ReportMerging #9599 will not alter performanceComparing Summary
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9599 +/- ##
==========================================
- Coverage 91.88% 91.84% -0.04%
==========================================
Files 433 433
Lines 26788 26798 +10
==========================================
- Hits 24613 24612 -1
- Misses 2175 2186 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -224,6 +224,32 @@ def _check_valid_triangular_parameters(prior: PriorDict) -> None: | |||
| ).set_context(self.name) | |||
| ) | |||
|
|
|||
| def _check_valid_derrf_parameters(prior: PriorDict) -> None: | |||
There was a problem hiding this comment.
It might be outside the scope for this PR, but did you consider converting this from a function to a dataclass? Something like:
@pydantic.dataclass
class DERRF:
name: str
steps: PositiveInt
min: float
max: float
skewness: float
width: confloat(gt=0)
@model_validator(mode="after")
def validate_min_max(self) -> Self:
if not self.max > self.min
raise ValueError(f"Max ({self.max}) must be larger than max ({self.max})
return self
def transform(self, x: float) -> float:
"""
Bin the result of `trans_errf` with `min=0` and `max=1` to closest of `nbins`
linearly spaced values on [0,1]. Finally map [0,1] to [min, max].
"""
q_values = np.linspace(start=0, stop=1, num=self.steps)
q_checks = np.linspace(start=0, stop=1, num=self.steps + 1)[1:]
y = TransformFunction.trans_errf(x, [0, 1, self.skewness, self.width])
bin_index = np.digitize(y, q_checks, right=True)
y_binned = q_values[bin_index]
result = self.min + y_binned * (self.max - self.min)
if result > self.max or result < self.min:
warnings.warn(
"trans_derff suffered from catastrophic loss of precision, clamping to min,max",
stacklevel=1,
)
return np.clip(result, self.min, self.max)
if np.isnan(result):
raise ValueError(
"trans_derrf returns nan, check that input arguments are reasonable"
)
return resultThere was a problem hiding this comment.
I did not, but will look into it.
There was a problem hiding this comment.
I guess this will require a storage migration?
There was a problem hiding this comment.
Yes, I guess it will, though it would be possible to work around it for now, and only migrate once we do something like this for all parameter types. If that is the direction you are heading with rewriting parameter configs?
Issue
Resolves #9523
Approach
Short description of the approach
(Screenshot of new behavior in GUI if applicable)
git rebase -i main --exec 'pytest tests/ert/unit_tests -n logical -m "not integration_test"')When applicable