-
Notifications
You must be signed in to change notification settings - Fork 109
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
Improve validation for unintialized ensembles in manual update and evaluate ensemble #8980
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8980 +/- ##
==========================================
- Coverage 91.54% 91.53% -0.01%
==========================================
Files 347 348 +1
Lines 21397 21433 +36
==========================================
+ Hits 19588 19619 +31
- Misses 1809 1814 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
9865892
to
683f3ee
Compare
134593d
to
ca70d8f
Compare
ca70d8f
to
24b160b
Compare
|
||
class RealizationsInEnsembleArgument(RangeStringArgument): | ||
UNINITIALIZED_REALIZATIONS_SPECIFIED = ( | ||
"The specified realization(s) %s are not found in selected ensemble." |
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.
Can we append the ensemble.name
?; ie. "The specified realization(s) %s are not found in selected ensemble %s."?
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.
Ignore :)
validation_status.setValue(token) | ||
return validation_status | ||
|
||
def _validate_selected_realization_exist(self, realization: int) -> bool: |
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.
Don't understand this one. Why we need to have responses when we are about to run evaluate experiment?
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.
^I meant to do parameters, not responses :)
24b160b
to
b029fee
Compare
attempted_realizations = rangestring_to_list(token) | ||
|
||
invalid_realizations = [] | ||
initialized_realization_ids = self.__ensemble.is_initalized() |
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.
now it makes sense :)
Can you squash the commits? |
experiment This commit adds validation for the realizations specified by the user when trying to run `manual_update` or `evaluate_experiment`. The validator checks the selected ensemble if the specified realization(s) exists. If not, the field becomes red and a warning is displayed. The commit also disables the realization field until an ensemble is selected.
d966047
to
79e6917
Compare
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.
Nice PR @jonathan-eq ! 🚀
Issue
Resolves #8691
Approach
Short description of the approach
(Screenshot of new behavior in GUI if applicable)
When specifying failed realizations:
git rebase -i main --exec 'pytest tests/ert/unit_tests -n logical -m "not integration_test"'
)When applicable