Skip to content
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

Smaller simplifications of everest baserunmodel #9158

Merged

Conversation

yngve-sk
Copy link
Contributor

@yngve-sk yngve-sk commented Nov 5, 2024

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2024

Codecov Report

Attention: Patch coverage is 28.57143% with 5 lines in your changes missing coverage. Please review.

Project coverage is 90.75%. Comparing base (1f00a17) to head (d5f9a80).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/ert/run_models/everest_run_model.py 28.57% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9158      +/-   ##
==========================================
- Coverage   90.75%   90.75%   -0.01%     
==========================================
  Files         351      351              
  Lines       21901    21903       +2     
==========================================
  Hits        19877    19877              
- Misses       2024     2026       +2     
Flag Coverage Δ
cli-tests 39.24% <0.00%> (-0.01%) ⬇️
gui-tests 71.74% <0.00%> (-0.01%) ⬇️
performance-tests 49.39% <0.00%> (+<0.01%) ⬆️
unit-tests 79.64% <28.57%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yngve-sk yngve-sk force-pushed the 24.11.05.simplify-everest-baserunmodel branch 2 times, most recently from 6e27ae1 to 4f7f6e1 Compare November 5, 2024 13:33
@yngve-sk yngve-sk force-pushed the 24.11.05.simplify-everest-baserunmodel branch from 4f7f6e1 to d5f9a80 Compare November 5, 2024 13:35
@yngve-sk yngve-sk marked this pull request as ready for review November 5, 2024 14:33
@yngve-sk yngve-sk self-assigned this Nov 5, 2024
Copy link
Contributor

@StephanDeHoop StephanDeHoop left a comment

Choose a reason for hiding this comment

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

Looks good to me, just have one comment/question

optimizer = self._configure_optimizer(simulator)

# Before each batch evaluation we check if we should abort:
optimizer.add_observer(
Copy link
Contributor

Choose a reason for hiding this comment

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

So functionally the only difference is that now we add an observer when we create the optimizer (at the end) instead of outside that method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. It was always created after initializing, just not within that function.

@yngve-sk yngve-sk merged commit 721f3c6 into equinor:main Nov 6, 2024
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants