Address PR comments

atlassian · Jul 22, 2024 · 92d2c11 · 92d2c11
1 parent 1d2eb85
commit 92d2c11
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 14 deletions.
diff --git a/...in/kotlin/com/atlassian/performance/tools/report/api/ShiftedDistributionRegressionTest.kt b/...in/kotlin/com/atlassian/performance/tools/report/api/ShiftedDistributionRegressionTest.kt
@@ -13,7 +13,6 @@ import org.apache.commons.math3.stat.descriptive.rank.Median
  * @experiment experiment durations
  * @param mwAlpha Mann-Whitney significance level
  * @param ksAlpha Kolmogorov-Smirnov significance level
- * @deprecated Use [DistributionComparator] instead
  */
 @Deprecated("Use DistributionComparator instead")
 class ShiftedDistributionRegressionTest(

diff --git a/.../kotlin/com/atlassian/performance/tools/report/api/distribution/DistributionComparator.kt b/.../kotlin/com/atlassian/performance/tools/report/api/distribution/DistributionComparator.kt
@@ -8,16 +8,15 @@ class DistributionComparator private constructor(
     private val baseline: DoubleArray,
     private val experiment: DoubleArray,
     /**
-     * A percentage by which experiment can be slower/faster than baseline and not considered as a regression/improvement
+     * A ratio by which experiment can be slower/faster than baseline and not considered as a regression/improvement
      */
     private val tolerance: Double,
     private val significance: Double
 ) {
 
 
-
     /**
-     * Performs a one-tailed Mann–Whitney U test to check whether experiment is not slower than the baseline
+     * Performs a one-tailed Mann–Whitney U test to check whether experiment is slower than the baseline
      *
      * @return true if the experiment is slower than the baseline by more than tolerance, false otherwise
      */
@@ -28,8 +27,7 @@ class DistributionComparator private constructor(
 
     private fun isExperimentImproved(baselineMedian: Double): Boolean {
         val mu = -tolerance * baselineMedian
-        val wilcoxon = WilcoxonRankSum(experiment, baseline, mu)
-        return wilcoxon.pValue1SidedLess < significance
+        return WilcoxonRankSum(experiment, baseline, mu).pValue1SidedLess < significance
     }
 
     /**
@@ -84,7 +82,7 @@ class DistributionComparator private constructor(
         val experimentRelativeChange = experimentRatio - 1
         return DistributionComparison(
             experimentRelativeChange = experimentRelativeChange,
-            experimentAbsoluteChange = experimentShift,
+            experimentShift = experimentShift,
             isExperimentRegressed = isExperimentRegressed,
             isExperimentImproved = isExperimentImproved
         )

diff --git a/.../kotlin/com/atlassian/performance/tools/report/api/distribution/DistributionComparison.kt b/.../kotlin/com/atlassian/performance/tools/report/api/distribution/DistributionComparison.kt
@@ -2,7 +2,7 @@ package com.atlassian.performance.tools.report.api.distribution
 
 class DistributionComparison(
     val experimentRelativeChange: Double,
-    val experimentAbsoluteChange: Double,
+    val experimentShift: Double,
     val isExperimentRegressed: Boolean,
     val isExperimentImproved: Boolean
 ) {

diff --git a/...com/atlassian/performance/tools/report/api/judge/RelativeNonparametricPerformanceJudge.kt b/...com/atlassian/performance/tools/report/api/judge/RelativeNonparametricPerformanceJudge.kt
@@ -77,7 +77,7 @@ class RelativeNonparametricPerformanceJudge private constructor(
         val impact = LatencyImpact.Builder(
             action,
             comparison.experimentRelativeChange,
-            reader.convertToDuration(comparison.experimentAbsoluteChange)
+            reader.convertToDuration(comparison.experimentShift)
         )
             .relevant(comparison.hasImpact())
             .build()

diff --git a/.../kotlin/com/atlassian/performance/tools/report/distribution/DistributionComparatorTest.kt b/.../kotlin/com/atlassian/performance/tools/report/distribution/DistributionComparatorTest.kt
@@ -35,7 +35,7 @@ class DistributionComparatorTest {
         // then
         plotQuantiles(baseline, experiment)
         assertSoftly {
-            it.assertThat(comparison.experimentAbsoluteChange).`as`("absolute change").isEqualTo(2.096466300542943E-4)
+            it.assertThat(comparison.experimentShift).`as`("absolute change").isEqualTo(2.096466300542943E-4)
             it.assertThat(comparison.experimentRelativeChange).`as`("relative change").isEqualTo(5.05322869548408E-7)
             it.assertThat(comparison.isExperimentImproved).`as`("improved").isFalse
             it.assertThat(comparison.isExperimentRegressed).`as`("regressed").isFalse
@@ -58,7 +58,7 @@ class DistributionComparatorTest {
         // then
         plotQuantiles(baseline, experiment)
         assertSoftly {
-            it.assertThat(comparison.experimentAbsoluteChange).`as`("absolute change").isEqualTo(800.0)
+            it.assertThat(comparison.experimentShift).`as`("absolute change").isEqualTo(800.0)
             it.assertThat(comparison.experimentRelativeChange).`as`("relative change").isEqualTo(1.9951194021713592)
             it.assertThat(comparison.isExperimentImproved).`as`("regressed").isFalse
             it.assertThat(comparison.isExperimentRegressed).`as`("regressed").isTrue
@@ -90,7 +90,7 @@ class DistributionComparatorTest {
         // then
         plotQuantiles(baseline, experiment)
         assertSoftly {
-            it.assertThat(comparison.experimentAbsoluteChange).`as`("absolute change").isEqualTo(66.57177057231809)
+            it.assertThat(comparison.experimentShift).`as`("absolute change").isEqualTo(66.57177057231809)
             it.assertThat(comparison.experimentRelativeChange).`as`("relative change").isEqualTo(0.16145163153504116)
             it.assertThat(comparison.isExperimentImproved).`as`("improved").isFalse
             it.assertThat(comparison.isExperimentRegressed).`as`("regressed").isTrue
@@ -127,7 +127,7 @@ class DistributionComparatorTest {
         // then
         plotQuantiles(baseline, experiment)
         assertSoftly {
-            it.assertThat(comparison.experimentAbsoluteChange).`as`("absolute change").isPositive
+            it.assertThat(comparison.experimentShift).`as`("absolute change").isPositive
             it.assertThat(comparison.experimentRelativeChange).`as`("relative change").isPositive
             it.assertThat(comparison.isExperimentImproved).`as`("improvement").isFalse
             it.assertThat(comparison.isExperimentRegressed).`as`("regressed").isTrue
@@ -186,7 +186,7 @@ class DistributionComparatorTest {
         val comparison = DistributionComparator.Builder(result, result).build().compare()
 
         // then
-        assertThat(comparison.experimentAbsoluteChange).isEqualTo(0.0)
+        assertThat(comparison.experimentShift).isEqualTo(0.0)
         assertThat(comparison.experimentRelativeChange).isEqualTo(0.0)
         assertThat(comparison.hasImpact()).isFalse()
     }