From d1b4d1ac115ef5de448bec5ae358475f2f9029a4 Mon Sep 17 00:00:00 2001
From: Alexander Nozik <altavir@gmail.com>
Date: Mon, 15 Feb 2021 09:32:25 +0300
Subject: [PATCH] Histograms refactor

---
 .../kmath/histogram/TreeHistogramSpace.kt     | 27 +++++++++----------
 .../kmath/histogram/UnivariateHistogram.kt    | 25 ++++++++---------
 2 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/kmath-histograms/src/jvmMain/kotlin/kscience/kmath/histogram/TreeHistogramSpace.kt b/kmath-histograms/src/jvmMain/kotlin/kscience/kmath/histogram/TreeHistogramSpace.kt
index b5872c601..67964fbeb 100644
--- a/kmath-histograms/src/jvmMain/kotlin/kscience/kmath/histogram/TreeHistogramSpace.kt
+++ b/kmath-histograms/src/jvmMain/kotlin/kscience/kmath/histogram/TreeHistogramSpace.kt
@@ -8,7 +8,7 @@ import java.util.*
 import kotlin.math.abs
 import kotlin.math.sqrt
 
-internal fun <B: ClosedFloatingPointRange<Double>> TreeMap<Double, B>.getBin(value: Double): B? {
+private fun <B : ClosedFloatingPointRange<Double>> TreeMap<Double, B>.getBin(value: Double): B? {
     // check ceiling entry and return it if it is what needed
     val ceil = ceilingEntry(value)?.value
     if (ceil != null && value in ceil) return ceil
@@ -29,12 +29,9 @@ public class TreeHistogram(
     override val bins: Collection<UnivariateBin> get() = binMap.values
 }
 
-private class UnivariateBinValue(
-    override val domain: UnivariateDomain,
-    override val value: Double,
-    override val standardDeviation: Double,
-) : UnivariateBin, ClosedFloatingPointRange<Double> by domain.range
-
+/**
+ * A space for univariate histograms with variable bin borders based on a tree map
+ */
 @UnstableKMathAPI
 public class TreeHistogramSpace(
     public val binFactory: (Double) -> UnivariateDomain,
@@ -73,7 +70,7 @@ public class TreeHistogramSpace(
         val resBins = TreeMap<Double, UnivariateBin>()
         bins.forEach { key, binCounter ->
             val count = binCounter.counter.value
-            resBins[key] = UnivariateBinValue(binCounter.domain, count, sqrt(count))
+            resBins[key] = UnivariateBin(binCounter.domain, count, sqrt(count))
         }
         return TreeHistogram(this, resBins)
     }
@@ -86,11 +83,13 @@ public class TreeHistogramSpace(
         require(b.context == this) { "Histogram $b does not belong to this context" }
         val bins = TreeMap<Double, UnivariateBin>().apply {
             (a.bins.map { it.domain } union b.bins.map { it.domain }).forEach { def ->
-                val newBin = UnivariateBinValue(
-                    def,
-                    value = (a[def.center]?.value ?: 0.0) + (b[def.center]?.value ?: 0.0),
-                    standardDeviation = (a[def.center]?.standardDeviation
-                        ?: 0.0) + (b[def.center]?.standardDeviation ?: 0.0)
+                put(def.center,
+                    UnivariateBin(
+                        def,
+                        value = (a[def.center]?.value ?: 0.0) + (b[def.center]?.value ?: 0.0),
+                        standardDeviation = (a[def.center]?.standardDeviation
+                            ?: 0.0) + (b[def.center]?.standardDeviation ?: 0.0)
+                    )
                 )
             }
         }
@@ -101,7 +100,7 @@ public class TreeHistogramSpace(
         val bins = TreeMap<Double, UnivariateBin>().apply {
             a.bins.forEach { bin ->
                 put(bin.domain.center,
-                    UnivariateBinValue(
+                    UnivariateBin(
                         bin.domain,
                         value = bin.value * k.toDouble(),
                         standardDeviation = abs(bin.standardDeviation * k.toDouble())
diff --git a/kmath-histograms/src/jvmMain/kotlin/kscience/kmath/histogram/UnivariateHistogram.kt b/kmath-histograms/src/jvmMain/kotlin/kscience/kmath/histogram/UnivariateHistogram.kt
index 87c56cf79..68b9019d0 100644
--- a/kmath-histograms/src/jvmMain/kotlin/kscience/kmath/histogram/UnivariateHistogram.kt
+++ b/kmath-histograms/src/jvmMain/kotlin/kscience/kmath/histogram/UnivariateHistogram.kt
@@ -10,26 +10,23 @@ import kscience.kmath.structures.asSequence
 
 public val UnivariateDomain.center: Double get() = (range.endInclusive - range.start) / 2
 
-public interface UnivariateBin : Bin<Double>, ClosedFloatingPointRange<Double> {
-    public val domain: UnivariateDomain
-
-    /**
-     * The value of histogram including weighting
-     */
-    public override val value: Double
-
-    /**
-     * Standard deviation of the bin value. Zero if not applicable
-     */
-    public val standardDeviation: Double
+/**
+ * A univariate bin based an a range
+ * @param value The value of histogram including weighting
+ * @param standardDeviation Standard deviation of the bin value. Zero or negative if not applicable
+ */
+public class UnivariateBin(
+    public val domain: UnivariateDomain,
+    override val value: Double,
+    public val standardDeviation: Double,
+) : Bin<Double>, ClosedFloatingPointRange<Double> by domain.range {
 
     public override val dimension: Int get() = 1
 
     public override fun contains(point: Buffer<Double>): Boolean = point.size == 1 && contains(point[0])
-
 }
 
-@UnstableKMathAPI
+@OptIn(UnstableKMathAPI::class)
 public interface UnivariateHistogram : Histogram<Double, UnivariateBin>,
     SpaceElement<UnivariateHistogram, Space<UnivariateHistogram>> {
     public operator fun get(value: Double): UnivariateBin?