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

Refactor summary collector #2802

Merged

Conversation

frode-aarstad
Copy link
Contributor

@frode-aarstad frode-aarstad commented Jan 27, 2022

Issue
Resolves #2735

Approach
Improves the performance by moving more of the code filling out the summary dataframe from python back to c++.

Pre review checklist

  • Added appropriate labels

Adding labels helps the maintainers when writing release notes, see sections and the
corresponding labels here: https://github.com/equinor/ert/blob/main/.github/release.yml

@codecov-commenter
Copy link

codecov-commenter commented Jan 27, 2022

Codecov Report

Merging #2802 (66aa583) into main (e52240b) will decrease coverage by 0.06%.
The diff coverage is 9.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2802      +/-   ##
==========================================
- Coverage   65.20%   65.14%   -0.07%     
==========================================
  Files         653      654       +1     
  Lines       53480    53515      +35     
  Branches     4791     4804      +13     
==========================================
- Hits        34874    34862      -12     
- Misses      17007    17050      +43     
- Partials     1599     1603       +4     
Impacted Files Coverage Δ
libres/lib/python/enkf_fs_summary_data.cpp 2.08% <2.08%> (ø)
res/enkf/export/summary_collector.py 100.00% <100.00%> (ø)
res/enkf/plot_data/ensemble_plot_data.py 54.05% <0.00%> (-27.03%) ⬇️
res/enkf/plot_data/ensemble_plot_data_vector.py 71.42% <0.00%> (-14.29%) ⬇️
libres/lib/enkf/block_fs_driver.cpp 81.35% <0.00%> (-0.57%) ⬇️
libres/lib/res_util/block_fs.cpp 53.67% <0.00%> (-0.45%) ⬇️
ert_gui/simulation/run_dialog.py 76.40% <0.00%> (+0.70%) ⬆️
ert_gui/ertwidgets/__init__.py 77.27% <0.00%> (+2.27%) ⬆️
ert_gui/ertwidgets/validationsupport.py 98.63% <0.00%> (+19.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e52240b...66aa583. Read the comment docs.

@frode-aarstad frode-aarstad marked this pull request as draft January 27, 2022 16:15
@frode-aarstad frode-aarstad force-pushed the refactor_summary_collector branch 3 times, most recently from b782c62 to fcbcea8 Compare January 27, 2022 20:22
@frode-aarstad frode-aarstad marked this pull request as ready for review January 28, 2022 09:05
@frode-aarstad frode-aarstad force-pushed the refactor_summary_collector branch 2 times, most recently from c1829e0 to c195e8a Compare February 3, 2022 10:11
libres/lib/python/enkf_fs_summary_data.cpp Outdated Show resolved Hide resolved
res/enkf/export/summary_collector.py Outdated Show resolved Hide resolved
Copy link
Contributor

@TerryHannant TerryHannant left a comment

Choose a reason for hiding this comment

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

Look good

@frode-aarstad frode-aarstad force-pushed the refactor_summary_collector branch 8 times, most recently from 66aa583 to 47737d4 Compare February 4, 2022 08:23
@frode-aarstad frode-aarstad merged commit 76ef3c4 into equinor:main Feb 4, 2022
@frode-aarstad frode-aarstad deleted the refactor_summary_collector branch February 4, 2022 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve performance of summary_collector
4 participants