Fix #1157. (#1162)

future_changes.md

@@ -6,3 +6,4 @@
 
 ### Fixed
 - "cmapmpl" color scale doesn't show the entire range of colors in continuous cmap. [[#1149](https://github.com/JetBrains/lets-plot/issues/1149)].
+- `geom_histogram`: wrong plot area when `y='..density..'` [[#1157](https://github.com/JetBrains/lets-plot/issues/1157)].

plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/stat/BinStatUtil.kt

@@ -74,9 +74,8 @@ object BinStatUtil {
         val (binCount, binWidth, startX) = getBinningParameters(rangeX, xPosKind, xPos, binOptions)
 
         // density plot area should be == 1
-        val normalBinWidth = rangeX.length / binCount
-        val densityNormalizingFactor = if (normalBinWidth > 0)
-            1.0 / normalBinWidth
+        val densityNormalizingFactor = if (binWidth > 0)
+            1.0 / binWidth
         else
             1.0
 

plot-base/src/commonTest/kotlin/org/jetbrains/letsPlot/core/plot/base/stat/BinStatUtilTest.kt

@@ -6,6 +6,9 @@
 package org.jetbrains.letsPlot.core.plot.base.stat
 
 import org.jetbrains.letsPlot.commons.interval.DoubleSpan
+import org.jetbrains.letsPlot.core.plot.base.DataFrame
+import org.jetbrains.letsPlot.core.plot.base.data.TransformVar
+import org.jetbrains.letsPlot.core.plot.base.stat.BinStat.Companion.DEF_BIN_COUNT
 import kotlin.test.*
 
 class BinStatUtilTest {
@@ -113,4 +116,48 @@ class BinStatUtilTest {
         assertContentEquals(listOf(1.0, 3.0), statData[Stats.Y_MAX])
         assertContentEquals(listOf(2.0, 3.0), statData[Stats.COUNT])
     }
+
+    @Test
+    fun checkComputeHistogramStatSeries() {
+        val valuesX = listOf(-0.5, 0.0, 0.0, 1.5)
+        val data = DataFrame.Builder()
+            .putNumeric(TransformVar.X, valuesX)
+            .build()
+        val statData = BinStatUtil.computeHistogramStatSeries(
+            data,
+            DoubleSpan(valuesX.min(), valuesX.max()),
+            valuesX,
+            BinStat.XPosKind.CENTER,
+            0.0,
+            BinStatUtil.BinOptions(DEF_BIN_COUNT, 0.5)
+        )
+        assertContentEquals(listOf(-0.5, 0.0, 0.5, 1.0, 1.5, 2.0), statData.x)
+        assertContentEquals(listOf(1.0, 2.0, 0.0, 0.0, 1.0, 0.0), statData.count)
+        assertContentEquals(listOf(0.5, 1.0, 0.0, 0.0, 0.5, 0.0), statData.density)
+    }
+
+    @Test
+    fun checkHistogramDensityArea() {
+        val checks = listOf(
+            listOf(0.0),
+            listOf(0.0, 1.0, 1.0),
+            listOf(-10.0, 0.0, 1.0, 1.0, 3.0),
+            listOf(0.0, 0.05, 0.051, 0.1),
+        )
+
+        for (valuesX in checks) {
+            val binOptions = BinStatUtil.BinOptions(DEF_BIN_COUNT, null)
+            val rangeX = DoubleSpan(valuesX.min(), valuesX.max())
+            val xPosKind = BinStat.XPosKind.NONE
+            val xPos = 0.0
+            val (_, binWidth, _) = BinStatUtil.getBinningParameters(rangeX, xPosKind, xPos, binOptions)
+            val data = DataFrame.Builder()
+                .putNumeric(TransformVar.X, valuesX)
+                .build()
+            val statData = BinStatUtil.computeHistogramStatSeries(data, rangeX, valuesX, xPosKind, xPos, binOptions)
+            val widthFactor = if (binWidth > 0) binWidth else 1.0
+            val area = widthFactor * statData.density.sum()
+            assertEquals(1.0, area, 1e-14)
+        }
+    }
 }

plot-base/src/jvmTest/kotlin/org/jetbrains/letsPlot/core/plot/base/stat/BinStatTest.kt

@@ -28,6 +28,15 @@ class BinStatTest {
         return statDf
     }
 
+    private fun getBinWidth(df: DataFrame, binCount: Int): Double {
+        val binOptions = BinStatUtil.BinOptions(binCount, null)
+        val statCtx = SimpleStatContext(df)
+        val rangeX = statCtx.overallXRange()
+        if (rangeX == null) return 1.0
+        val (_, binWidth, _) = BinStatUtil.getBinningParameters(rangeX, BinStat.XPosKind.NONE, 0.0, binOptions)
+        return binWidth
+    }
+
     @Test
     fun twoPointsInOneBin() {
         val df = DataFrameUtil.fromMap(
@@ -37,12 +46,13 @@ class BinStatTest {
         )
 
         val statDf = applyBinStat(df, 1)
+        val binWidth = getBinWidth(df, 1)
 
         // expecting count = [2]
         assertThat(statDf.getNumeric(Stats.COUNT), Matchers.contains(2.0))
 
-        // expecting density = [1]
-        assertThat(statDf.getNumeric(Stats.DENSITY), Matchers.contains(1.0))
+        // expecting density = [1 / width]
+        assertThat(statDf.getNumeric(Stats.DENSITY), Matchers.contains(1.0 / binWidth))
     }
 
     @Test
@@ -54,12 +64,14 @@ class BinStatTest {
         )
 
         val statDf = applyBinStat(df, 2)
+        val binWidth = getBinWidth(df, 2)
 
         // expecting count = [1, 1]
         assertThat(statDf.getNumeric(Stats.COUNT), Matchers.contains(1.0, 1.0))
 
-        // expecting density = [1, 1]   (width = 0.5 -> 0.5 + 0.5 = 1)
-        assertThat(statDf.getNumeric(Stats.DENSITY), Matchers.contains(1.0, 1.0))
+        // expecting density sum is equal to 1 / width
+        val area = binWidth * statDf.getNumeric(Stats.DENSITY).filterNotNull().sum()
+        assertThat(area, Matchers.closeTo(1.0, 1e-12))
     }
 
     @Test
@@ -71,11 +83,13 @@ class BinStatTest {
         )
 
         val statDf = applyBinStat(df, 4)
+        val binWidth = getBinWidth(df, 4)
 
         // expecting count = [1,0,0,1]
         assertThat(statDf.getNumeric(Stats.COUNT), Matchers.contains(1.0, 0.0, 0.0, 1.0))
 
-        // expecting density = [2, 0, 0, 2]  (width = 0.25 -> 2 * 0.25 + 0 + 0 + 2 * 0.25 = 1)
-        assertThat(statDf.getNumeric(Stats.DENSITY), Matchers.contains(2.0, 0.0, 0.0, 2.0))
+        // expecting density sum is equal to 1 / width
+        val area = binWidth * statDf.getNumeric(Stats.DENSITY).filterNotNull().sum()
+        assertThat(area, Matchers.closeTo(1.0, 1e-12))
     }
 }