Skip to content

Create distribution plot - #13985

Open
eilskra wants to merge 4 commits into
equinor:mainfrom
eilskra:distribution-plot
Open

Create distribution plot#13985
eilskra wants to merge 4 commits into
equinor:mainfrom
eilskra:distribution-plot

Conversation

@eilskra

@eilskra eilskra commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Issue
Resolves #13952
Resolves #13769
Resolves #13950

Approach
We had three ways of plotting the distribution, Gaussian KDE, Histogram and Distribution. To improve the visualization have attempted to combine these and introduce more customizbility to the user. The assumption made for the use-case is that it is first and last iterations that is the most interesting (ensemble selected = iter-0and iter-3)

Histogram:

  • No longer displays fixed bins, as this could create empty bins essentially only consisting of white space. Now uses the built in matplotlib srt functionality. Similar to original code, but can lead to different bin width Use matplotlib method for bin creation for histogram #13961
  • No longer displays the histograms for each ensemble stacked above one another, but rather overlaps. Works well with the assumed use case (though could benefit from different palette), but if many ensembles toggled will become muddy

Gaussian KDE:
Works similar as before, but now potentially plotted on top of the histogram. Does not allow log scale. Changing scale post evaluation looks off

Rug plot (old distribution plot):
No longer scatter plot in a single line, now rug plots underneath the "main plot" (contains histogram and the gkde). Illustrates groupings better. Is displayed on the x axis rather than y and shares axis as the "main plot", works with log-scale as before. Decided not to have a shared axis for better readability and to illustrate the ensembles grouping.
Note: Similar to histogram, could benefit from another palette as yellow is hard to read

No handling of categorical data, so could not remove HIstogramPlot as of now, only GaussianKDE

Screenshot 2026-08-19 at 15 32 29 all_options gkde_individual Screenshot 2026-08-19 at 14 59 19 hist_individual
  • 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')

Copilot AI left a comment

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.

Pull request overview

This PR updates the ERT GUI plotting to consolidate histogram/KDE functionality into a single Distribution plot, adding plot-option controls to stack multiple ensembles in one view (with opacity) rather than using separate plot tabs.

Changes:

  • Replace separate “Histogram” and “Gaussian KDE” plot tabs with a unified Distribution plot supporting histogram/KDE/rug options and density vs count mode.
  • Add a new DistributionOptions control widget and wire its state into PlotContext via PlotWindow.fetch_data().
  • Refactor PlotTools helpers (snake_case naming + small utility methods) and adjust plot layout engine configuration.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/ert/gui/plotting/widgets/plot_widget.py Switch Matplotlib layout engine to constrained.
src/ert/gui/plotting/widgets/plot_controls/distribution_options.py New plot-control widget for distribution plot options (histogram/KDE/rug + density/count).
src/ert/gui/plotting/widgets/plot_controls/init.py Export DistributionOptions.
src/ert/gui/plotting/utils/plot_tools.py Rename helpers to snake_case and add set_title, set_labels_for_axes_from_context, remove_spines.
src/ert/gui/plotting/utils/plot_context.py Add distribution-plot state to PlotContext (histogram/rug/KDE/by_density).
src/ert/gui/plotting/plot_window.py Remove histogram/KDE tabs; add DistributionOptions widget and propagate its state into PlotContext.
src/ert/gui/plotting/ert_plots/distribution.py Implement the new stacked distribution plot (histogram/KDE/rug; density vs count).

Comment thread src/ert/gui/plotting/widgets/plot_controls/distribution_options.py Outdated
Comment thread src/ert/gui/plotting/widgets/plot_controls/distribution_options.py Outdated
Comment thread src/ert/gui/plotting/widgets/plot_controls/distribution_options.py Outdated
Comment thread src/ert/gui/plotting/ert_plots/distribution.py Outdated
Comment thread src/ert/gui/plotting/ert_plots/distribution.py Outdated
Comment thread src/ert/gui/plotting/ert_plots/distribution.py Outdated
@eilskra
eilskra force-pushed the distribution-plot branch 8 times, most recently from 1d37447 to 9c17929 Compare July 13, 2026 13:57
@eilskra
eilskra requested a review from Copilot July 13, 2026 14:19

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 8 comments.

Comments suppressed due to low confidence (1)

src/ert/gui/plotting/plot_window.py:38

  • ert.gui.plotting.ert_plots is still re-exporting GaussianKDEPlot/HistogramPlot from modules that are deleted in this PR, which will raise ImportError when this file imports DistributionPlot from that package. Update src/ert/gui/plotting/ert_plots/__init__.py (and any remaining references) to stop importing/re-exporting the removed plots.
from ert.gui.plotting.ert_plots import (
    CrossEnsembleStatisticsPlot,
    DistributionPlot,
    MisfitsPlot,
    StatisticsPlot,
    StdDevPlot,
)

Comment thread src/ert/gui/plotting/plot_window.py Outdated
Comment thread src/ert/gui/plotting/ert_plots/distribution.py
Comment thread src/ert/gui/plotting/ert_plots/distribution.py
Comment thread src/ert/gui/plotting/ert_plots/distribution.py Outdated
Comment thread src/ert/gui/plotting/ert_plots/distribution.py Outdated
Comment thread src/ert/gui/plotting/ert_plots/distribution.py
Comment thread src/ert/gui/plotting/ert_plots/distribution.py
Comment thread src/ert/gui/plotting/ert_plots/distribution.py
@eilskra
eilskra force-pushed the distribution-plot branch 4 times, most recently from 138bffd to d91fadb Compare July 14, 2026 07:17
@eilskra
eilskra requested a review from Copilot July 14, 2026 07:18
@codecov-commenter

codecov-commenter commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.78543% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.99%. Comparing base (293883c) to head (8f2c20b).

Files with missing lines Patch % Lines
...ting/widgets/plot_controls/distribution_options.py 94.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13985      +/-   ##
==========================================
+ Coverage   91.97%   91.99%   +0.01%     
==========================================
  Files         484      484              
  Lines       33644    33798     +154     
==========================================
+ Hits        30945    31093     +148     
- Misses       2699     2705       +6     
Flag Coverage Δ
cli-tests 36.25% <0.00%> (-0.17%) ⬇️
fuzz 44.32% <28.34%> (-0.07%) ⬇️
gui-tests 58.79% <85.82%> (+0.08%) ⬆️
performance-and-unit-tests 81.06% <98.78%> (+0.08%) ⬆️
test 45.94% <34.81%> (-0.03%) ⬇️

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% <ø> (ø)
src/ert/gui/plotting/ert_plots/distribution.py 100.00% <100.00%> (+14.54%) ⬆️
src/ert/gui/plotting/plot_window.py 78.88% <100.00%> (+0.18%) ⬆️
src/ert/gui/plotting/utils/plot_context.py 93.05% <100.00%> (+1.18%) ⬆️
src/ert/gui/plotting/utils/plot_maps.py 100.00% <ø> (ø)
src/ert/gui/plotting/utils/plot_tools.py 100.00% <100.00%> (ø)
...ert/gui/plotting/widgets/plot_controls/__init__.py 100.00% <100.00%> (ø)
src/ert/gui/plotting/widgets/plot_widget.py 88.31% <100.00%> (+0.15%) ⬆️
...ting/widgets/plot_controls/distribution_options.py 94.00% <94.00%> (ø)

... and 3 files with indirect coverage changes

@eilskra
eilskra force-pushed the distribution-plot branch from d91fadb to 6a40c4f Compare July 14, 2026 07:24

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.

Comment thread src/ert/gui/plotting/ert_plots/distribution.py
Comment thread src/ert/gui/plotting/ert_plots/distribution.py Outdated
Comment thread src/ert/gui/plotting/ert_plots/distribution.py
Comment on lines +76 to +78
self._plot_distribution(
figure, plot_context, ensemble_to_data_map, plot_context.by_density
)
@ertomatic

Copy link
Copy Markdown
Collaborator

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

@eilskra
eilskra force-pushed the distribution-plot branch 2 times, most recently from 206f834 to 03f25b8 Compare July 14, 2026 07:43
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 14, 2026
@eilskra eilskra self-assigned this Jul 17, 2026
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 17, 2026
@eilskra
eilskra marked this pull request as draft July 20, 2026 07:08
@eilskra

eilskra commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Marking as blocked until GKDE clarification

@eilskra eilskra added blocked release-notes:improvement Automatically categorise as improvement in release notes and removed improvement Something nice to have, that will make life easier for developers or users or both. labels Jul 21, 2026
@eilskra
eilskra force-pushed the distribution-plot branch from 9eec127 to c731588 Compare August 12, 2026 14:18
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Aug 12, 2026
@ertomatic

Copy link
Copy Markdown
Collaborator

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

@eilskra
eilskra force-pushed the distribution-plot branch 4 times, most recently from 91c74f3 to adb112e Compare August 13, 2026 11:52

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

src/ert/gui/plotting/ert_plots/distribution.py:94

  • With Histogram unchecked and Gaussian KDE checked, this still creates a twin histogram axis. _scale_count_axes() and _finalize_axes() then show and scale a Count (Histogram) axis even though no histogram was requested. Gate twin-axis creation, count-axis scaling, and the dual labels on both options being enabled; a KDE-only plot should use only the density axis.
        use_twin_axes = self._gkde_plot
        histogram_axes = main_axes.twinx() if use_twin_axes else main_axes

src/ert/gui/plotting/widgets/plot_widget.py:156

  • The PR states that it resolves #13769 by reducing plotter padding, but this file still leaves the QVBoxLayout default margins/spacing in place and retains the explicit 8-pixel spacer. The added reset logic does not address that issue; apply the zero-margin/zero-spacing layout change described in #13769.
        # Some figures contain twinaxes
        # Resetting the xscale to linear for all axes
        # to avoid log scale issues when re-plotting after a log scale plot

src/ert/gui/plotting/widgets/plot_controls/distribution_options.py:60

  • This introduces user-facing distribution controls and substantially changes the Distribution plot, but no corresponding .rst user documentation was added or updated. Add usage documentation for the three toggles and explain the replacement/removal of the standalone Gaussian KDE behavior, as required for user-facing changes in this repository.
        self._distribution_options = CollapsibleSection(
            "Distribution options",
            create_group_layout(
                [
                    self._histogram,
                    self._gkde,
                    self._rug_plot,
                ]
            ),
            expanded=True,

Comment thread tests/ert/ui_tests/gui/test_plotting_of_snake_oil.py Outdated
@berland

berland commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

I suggest the following changes to the tick-boxes:
"Show gkde" -> "Show estimated density"
"Show rug plot" -> "Show individual points" (I am not sure I have heard about the term 'rug plot' before)

@berland

berland commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

̃Does this not replace the Histogram tab? So it should be removed? I see the comment on categorical data now.

@berland

berland commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

It could be a tick option to have stacked histogram plots, but that is fine to save for a different PR (more valid if the Histogram tab should be removed)

@eilskra

eilskra commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

It could be a tick option to have stacked histogram plots, but that is fine to save for a different PR (more valid if the Histogram tab should be removed)

@berland Stacked as in the previous functionality or as in stacked in same figure?

@eqbech eqbech left a comment

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.

Should we maybe reduce Y-padding a bit between the rug-plots?

Image

Comment thread src/ert/gui/plotting/ert_plots/distribution.py Outdated
Comment thread tests/ert/unit_tests/gui/plotting/ert_plots/test_distribution_plot.py Outdated
Comment thread tests/ert/unit_tests/gui/plotting/widgets/test_distribution_options.py Outdated
@berland

berland commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

@berland Stacked as in the previous functionality or as in stacked in same figure?

As in previous functionality, to easen up removal of the (old) tab.

Preperations for the new plot.

The different options will allow the user to toggle
the plot types to add to the figure and what type
of axis to display. Each plot option contains helper text

Intention is to replace the single line scatter-points with rug plot to
better visualize the distribution
Prep. for new DistributionPlot

The DistributionPlot will not be able to use finalizePlot(),
due to it having multiple axis and grid.

Want to allow users to modify the figure, e.g changing labels, toggle grid etc.
Extracting methods from finalizePlot() to standalone methods, such that
implementation can try to align as much as possible with the finalizePlot()-flow
Prep. for new DistributionPlot

The new DistributionPlot will use multiple axis, so need to be able to reset all axes
at reset(). Otherwise will get log-scale error when un-checking the log-scale checkbox.
DistributionPlot implementation

Removed GKDE, including references in plot_window-tests

Added DistributionPlot and corresponding tests. The plot implements
three methods of plotting the distribution:
- Gaussian KDE
- Overlapping Histogram (y-axis can be set to count or density)
- Rug plots for to visualize the raw datapoints.

Behaviour of the plot changes depending of what plots are toggled.
If GKDE or Hist. is turned off, the rugplots will act as the main window
otherwise, they will be a supporting plot underneath the "main" plot-window

If multiple ensembles are selected the overlapping histograms can become muddied
and hard to distinquish. This is not a common use-case as most users will only select
first and last.

No screenshot-test added, only unit testing. Plot will likely undergo refactoring/updates,
so will await screenshot-testing until "stable".

Note: Cannot remove original HistogramPlot as of now, as the new plot can not
handle categorical data yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes:improvement Automatically categorise as improvement in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stack histogram rather than multiple individual plots Combine distribution, Histogram and Gaussian KDE Reduce padding around Ert plotter

8 participants