Skip to content

#7750 Histogram Plot: Add logarithmic binning and user-defined bin range - #14512

Merged
magnesj merged 4 commits into
devfrom
7750-histogram-logarithmic-binning
Aug 13, 2026
Merged

#7750 Histogram Plot: Add logarithmic binning and user-defined bin range#14512
magnesj merged 4 commits into
devfrom
7750-histogram-logarithmic-binning

Conversation

@kriben

@kriben kriben commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Add BinningMode (Linear/Logarithmic) and OutOfRangeHandling (Exclude/Include in Boundary Bins) to RigHistogramCalculator, with defaulted constructor arguments so existing callers are unaffected. All four histogram plot data sources get shared options for binning mode and an optional user-defined bin range through new PDM fields on RimHistogramDataSource.

The grid statistics data source computes non-default histograms through a new non-caching RigStatisticsDataCache::computeHistogram() pass-through, leaving the cached statistics used by the 3D overlay and the P10/P90/mean annotation lines untouched.

Selecting logarithmic binning enables logarithmic scale on the plot X axis once, via a new signal handled by RimHistogramCurve.

Also fix the bin index computation to use uniform bins of width range/nBins, consistent with calculatePercentil() and the bin edges drawn by the plots. The previous divisor (nBins - 1) shifted counts by up to one bin width relative to the drawn bars and slightly biased histogram-based percentile estimates.

Fixes #7750.
Fixes #12720.

Add BinningMode (Linear/Logarithmic) and OutOfRangeHandling (Exclude/Include in
Boundary Bins) to RigHistogramCalculator, with defaulted constructor arguments so
existing callers are unaffected. All four histogram plot data sources get shared
options for binning mode and an optional user-defined bin range through new PDM
fields on RimHistogramDataSource.

The grid statistics data source computes non-default histograms through a new
non-caching RigStatisticsDataCache::computeHistogram() pass-through, leaving the
cached statistics used by the 3D overlay and the P10/P90/mean annotation lines
untouched.

Selecting logarithmic binning enables logarithmic scale on the plot X axis once,
via a new signal handled by RimHistogramCurve.

Also fix the bin index computation to use uniform bins of width range/nBins,
consistent with calculatePercentil() and the bin edges drawn by the plots. The
previous divisor (nBins - 1) shifted counts by up to one bin width relative to
the drawn bars and slightly biased histogram-based percentile estimates.
@kriben
kriben marked this pull request as ready for review August 12, 2026 12:43
@kriben
kriben requested a review from magnesj August 12, 2026 12:43
@kriben kriben self-assigned this Aug 12, 2026
Comment on lines +141 to 143
EXPECT_DOUBLE_EQ( 7292.3587591482656, p50 );
EXPECT_DOUBLE_EQ( 96798.640494338761, p90 );
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Discussion with claude regarding this:

 Why is the RigStatisticsMath.HistogramPercentiles test changed?                                                                                                                                                                                              

● Because it hard-codes the exact P10/P50/P90 estimates that RigHistogramCalculator produces for a fixed data set, and those estimates depend on how values are assigned to bins — which is the one deliberate behavior change in this branch.

  The old addValue computed the bin index as

  index = (size_t)( m_maxIndex * ( value - m_min ) / m_range );   // divisor = nBins - 1

  i.e. bins of width range/(nBins-1), with the last bin effectively catching only value == max. But calculatePercentil() — the function this test exercises — interpolates assuming uniform bins of width range/nBins (binWidth = m_range / 
  m_histogram->size()), and the plots draw edges with that same range/nBins spacing. So the histogram the percentile math read was systematically shifted relative to the bin geometry it assumed. When you approved fixing the divisor to nBins (index = 
  min(floor(nBins·(v-min)/range), nBins-1)), a few of the 13 valid samples in the test moved into adjacent bins, so the interpolated estimates moved too:

  - P50: 5312.131… → 7292.359…
  - P90: 94818.413… → 96798.640…
  - P10: unchanged (-76273.240…) — that sample happened to land in the same bin under both formulas, and I verified the old and new expected values are bit-identical doubles there.

  The new expectations weren't copied from the test's own output: I recomputed them independently in Python by reimplementing the documented formula (bin counts with the nBins divisor, then the cumulative-count interpolation described in the function's
  comment block) over the same 17-value input, and used those full-precision results in the EXPECT_DOUBLE_EQs. So the test still checks the same property — histogram-based percentile estimation on a fixed dataset — just against the corrected, internally
  consistent binning.

  The same shift is why I flagged that statistics-case P10/P90 results and the 3D overlay's histogram-based percentile fallback may show small diffs in regression baselines.

kriben added 3 commits August 12, 2026 15:16
Move Number of Bins and the new binning fields into a shared UI group created by
RimHistogramDataSource::appendBinningUiOrdering().
…i plot

Add a virtual RimMultiPlot::handleDroppedObjects so RiuMultiPlotBook routes drop
events to any multi plot type instead of only RimSummaryMultiPlot. Implement the
handler in RimHistogramMultiPlot: each dropped ensemble summary vector, ensemble
parameter or file set ensemble creates its own new histogram subplot, following
the behavior of the summary multi plot. Extract RimHistogramPlot::isDroppableObject
so unsupported dropped objects do not create empty subplots.
…emble

When an ensemble is dropped on a histogram plot showing summary vector curves,
create a new curve per distinct vector and time step for the dropped ensemble,
matching the existing behavior for ensemble parameter curves.
@magnesj
magnesj merged commit 5674130 into dev Aug 13, 2026
24 of 25 checks passed
@magnesj
magnesj deleted the 7750-histogram-logarithmic-binning branch August 13, 2026 05:19
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.

Histogram: add drag'n drop for Summary Vector Histogram Plot: Logarithmic binning for some results

2 participants