-
Notifications
You must be signed in to change notification settings - Fork 29
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
48 simplify notch approximation lawbinned #60
48 simplify notch approximation lawbinned #60
Conversation
Signed-off-by: Benjamin Maier <[email protected]>
Signed-off-by: Benjamin Maier <[email protected]>
Signed-off-by: Benjamin Maier <[email protected]>
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.
Quite an improvement, thanks. Please take a look at the one comment in the code.
index = self._lut_primary_branch.load.searchsorted(np.abs(load.values))-1 # "-1", transform to zero-based indices | ||
|
||
# raise error if requested load is higher than initialized maximum absolute load | ||
if np.any(index+1 >= len(self._lut_primary_branch)): | ||
raise ValueError(f"Binned class is initialized with a maximum absolute load of {self._maximum_absolute_load}, "\ | ||
f" but a higher absolute load value of |{load.max()}| is requested (in stress()).") |
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.
Maybe we can ._find_load_index_or_raise(load)
which finds the index
of the load or raises if any of them is outside. Would remove code duplication and help readability.
Or at least introduce a method ._raise_if_requested_load_higher_than_max_abs_load(index, max_abs_load, max_load)
We should merge #67 ASAP to get the CI working again |
Signed-off-by: Benjamin Maier <[email protected]>
…o 48-simplify-notch_approximation_lawbinned Signed-off-by: Benjamin Maier <[email protected]>
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.
Squash?
I forgot the Sign-off again. Does it gets fixed with a squash? Then yes, please :-) |
Ok, thanks. |
No description provided.