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

Avoid BoolVector in Python code #3251

Merged
merged 2 commits into from
Apr 21, 2022

Conversation

berland
Copy link
Contributor

@berland berland commented Apr 11, 2022

Issue
Resolves #3121
Resolves #3141

Approach
Replace usage of BoolVector with List[bool]except for the final call to wrapped C-code. Use new Python object ActiveRange for interpreting range strings.

Pre review checklist

  • Added appropriate release note label
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Commit history is consistent and clean, in line with the contribution guidelines.

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

@berland berland marked this pull request as draft April 11, 2022 14:03
@berland berland added the release-notes:maintenance Automatically categorise as maintenance change in release notes label Apr 12, 2022
@berland berland self-assigned this Apr 12, 2022
@berland berland linked an issue Apr 12, 2022 that may be closed by this pull request
@berland berland changed the title Use Python for managing active masks Avoid BoolVector in Python code Apr 12, 2022
@berland berland marked this pull request as ready for review April 19, 2022 07:43
@berland berland marked this pull request as draft April 19, 2022 08:02
@berland berland force-pushed the port_handling_active_range branch 5 times, most recently from 410f10b to 6bbb828 Compare April 19, 2022 12:08
@berland berland marked this pull request as ready for review April 19, 2022 12:19
@berland berland marked this pull request as draft April 19, 2022 13:01
@berland berland marked this pull request as ready for review April 20, 2022 08:18
@@ -83,7 +84,7 @@ def run_path(self):
return self._enkf_main.getModelConfig().getRunpathAsString()

def load_from_forward_model(
self, case: str, realisations: BoolVector, iteration: int
self, case: str, realisations: List[bool], iteration: int
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that we use both spelling for realization and realisation. Maybe we should unify them at same point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that would be an issue of its own.

@berland berland force-pushed the port_handling_active_range branch 3 times, most recently from 6594c70 to 812b5cd Compare April 20, 2022 11:40
Copy link
Contributor

@xjules xjules left a comment

Choose a reason for hiding this comment

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

Good job! LGTM 🚀

@codecov-commenter
Copy link

codecov-commenter commented Apr 20, 2022

Codecov Report

Merging #3251 (f5655f3) into main (0720548) will decrease coverage by 0.00%.
The diff coverage is 81.60%.

@@            Coverage Diff             @@
##             main    #3251      +/-   ##
==========================================
- Coverage   64.90%   64.90%   -0.01%     
==========================================
  Files         619      619              
  Lines       48709    48647      -62     
  Branches     4370     4370              
==========================================
- Hits        31617    31572      -45     
+ Misses      15652    15632      -20     
- Partials     1440     1443       +3     
Impacted Files Coverage Δ
ert/ensemble_evaluator/activerange.py 97.67% <ø> (ø)
ert_gui/simulation/run_dialog.py 81.53% <ø> (+0.69%) ⬆️
ert_gui/simulation/single_test_run_panel.py 95.65% <0.00%> (-0.19%) ⬇️
ert_shared/models/iterated_ensemble_smoother.py 88.34% <0.00%> (ø)
ert_gui/ertwidgets/models/ertmodel.py 50.00% <20.00%> (-2.95%) ⬇️
res/enkf/plot_data/ensemble_plot_gen_kw.py 51.78% <28.57%> (-1.92%) ⬇️
res/enkf/enkf_main.py 82.09% <70.00%> (-0.50%) ⬇️
res/enkf/ert_run_context.py 94.68% <88.88%> (+0.23%) ⬆️
res/enkf/state_map.py 92.50% <88.88%> (-0.19%) ⬇️
...t_gui/ertwidgets/models/activerealizationsmodel.py 91.30% <100.00%> (ø)
... and 15 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@berland
Copy link
Contributor Author

berland commented Apr 21, 2022

jenkins test pleas

@berland berland merged commit b037aff into equinor:main Apr 21, 2022
@berland berland deleted the port_handling_active_range branch June 15, 2022 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:maintenance Automatically categorise as maintenance change in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid using bool_vector in simulation arguments Port handling of active realization masks to python
4 participants