Skip to content

Add waterfall plot - #13921

Open
xjules wants to merge 5 commits into
equinor:mainfrom
xjules:enif_plot
Open

Add waterfall plot#13921
xjules wants to merge 5 commits into
equinor:mainfrom
xjules:enif_plot

Conversation

@xjules

@xjules xjules commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Issue
Resolves #13730

image

Approach
Short description of the approach

(Screenshot of new behavior in GUI if applicable)

  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

@xjules xjules self-assigned this Jul 1, 2026
Comment thread src/ert/gui/plotting/waterfall_data.py Outdated
@codecov-commenter

codecov-commenter commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 55.98291% with 103 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.67%. Comparing base (47757c5) to head (c800d59).
⚠️ Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
src/ert/gui/plotting/ert_plots/waterfall.py 18.75% 78 Missing ⚠️
src/ert/gui/plotting/plot_api.py 50.00% 7 Missing ⚠️
src/ert/dark_storage/endpoints/ensembles.py 53.84% 6 Missing ⚠️
src/ert/gui/plotting/plot_window.py 64.70% 6 Missing ⚠️
src/ert/gui/plotting/waterfall_data.py 93.47% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13921      +/-   ##
==========================================
- Coverage   91.93%   91.67%   -0.27%     
==========================================
  Files         482      484       +2     
  Lines       33516    33748     +232     
==========================================
+ Hits        30813    30938     +125     
- Misses       2703     2810     +107     
Flag Coverage Δ
cli-tests 36.16% <0.00%> (-0.32%) ⬇️
fuzz 44.25% <16.23%> (-0.21%) ⬇️
gui-tests 58.55% <16.23%> (-0.30%) ⬇️
performance-and-unit-tests 80.68% <52.99%> (-0.21%) ⬇️
test 45.77% <16.23%> (-0.22%) ⬇️

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

Files with missing lines Coverage Δ
src/ert/gui/plotting/ert_plots/__init__.py 100.00% <100.00%> (ø)
src/ert/gui/plotting/utils/plot_maps.py 100.00% <100.00%> (ø)
src/ert/dark_storage/endpoints/ensembles.py 85.00% <53.84%> (-15.00%) ⬇️
src/ert/gui/plotting/plot_window.py 77.75% <64.70%> (-0.55%) ⬇️
src/ert/gui/plotting/waterfall_data.py 93.47% <93.47%> (ø)
src/ert/gui/plotting/plot_api.py 78.84% <50.00%> (-1.36%) ⬇️
src/ert/gui/plotting/ert_plots/waterfall.py 18.75% <18.75%> (ø)

... and 3 files with indirect coverage changes

@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#45

@xjules
xjules marked this pull request as ready for review July 9, 2026 07:49
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 9, 2026
@codspeed-hq

codspeed-hq Bot commented Jul 9, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing xjules:enif_plot (dd1ffc7) with main (fba40ad)

Open in CodSpeed

ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 9, 2026
fontweight="bold",
)

figure.tight_layout()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The layout engine sets this by default, so not needed. Can lead to unwanted formatting-behaviour when resizing the window

Comment on lines +174 to +188
ax.set_xticks(x)
ax.set_xticklabels(names, rotation=45, ha="right", fontsize=8)
ax.set_ylabel("Mean parameter value (standardized)", fontsize=10)
ax.axhline(0, color="black", linewidth=0.5)
ax.grid(axis="y", alpha=0.3)

key = plot_context.key()
experiment_name = truncate_experiment_name(ensemble.experiment_name)
n_contrib = int(contrib_mask.sum())
ax.set_title(
f"Expected update for {key}\n"
f"by top {n_contrib} observation contributions"
f"\n({experiment_name} : {ensemble.name})",
fontsize=12,
fontweight="bold",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some of this could be covered byPlotTools.finalizePlot() and setting the PlotType as BOX or BAR.

That will also allow the user to update the plot, i.e toggle grid/legend, changing title and labels

Comment on lines +19 to +20
COLOR_POSITIVE = "#70AD47"
COLOR_NEGATIVE = "#ED7D31"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like the idea of having specific color to convey info, I think other plots (especially Everest) could benefit from aligning with these colors, so its consistent across the application.

Could we move this to a central file, maybe in the plot utils or plot_tools, such that we could use this as global palette for positive/improvements and negative/rejected?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, sounds like a good idea

_plot_waterfall(figure, plot_context, data, ensemble)


def _plot_waterfall(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will there be more plot-variants of this plot, e.g _plot_waterfall_with_x_condition()?

If not, could this be moved to the plot-function instead of being its own function?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, not at the moment at least.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We do use the following pattern though: eg. Histogram.plot(...) -> plotHistogram(...)

va=va,
fontsize=7,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we add the color of positive/negative to the legend to help guide the user?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hm, maybe an idea.
I was trying to reproduce the plots used in the assets. I can try to poke some expert users if this would make sense.

if not ensemble_to_data_map:
return

# Find the first ensemble that has non-empty waterfall data

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What happens if the user re-organizes the ensemble selection? e.g moving iter-0 after iter-1, will the behavior work as intended then?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if multiple ensembles are selected, (iter-0 - iter-3), would the user know why or how to get the "correct" ensemble to plot?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this plot should be primarily dedicated to show enif (transition between prior to posterior). So if we would select multiple ensembles that would be only to compare two experiments with enif.
I haven't got that far yet :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Enif plot will only support one ensemble for the time being.

ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 14, 2026
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 15, 2026
xjules added 2 commits August 18, 2026 15:59
Use polars instead of pandas
Make sure that blob_type==BlobType.MATRIX
Add unit test for waterfall data computations
Make only one ensemble selecteble
@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#86

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.

Explainability: Implement Waterfall plot for enif

4 participants