Use new significance algorithm by default

Author: Mark-Simulacrum

site/src/comparison.rs

@@ -48,7 +48,6 @@ pub async fn handle_triage(
             "instructions:u".to_owned(),
             ctxt,
             &master_commits,
-            body.calcNewSig.unwrap_or(false),
         )
         .await
         .map_err(|e| format!("error comparing commits: {}", e))?
@@ -97,17 +96,11 @@ pub async fn handle_compare(
         .await
         .map_err(|e| format!("error retrieving master commit list: {}", e))?;
     let end = body.end;
-    let comparison = compare_given_commits(
-        body.start,
-        end.clone(),
-        body.stat,
-        ctxt,
-        &master_commits,
-        body.calcNewSig.unwrap_or(false),
-    )
-    .await
-    .map_err(|e| format!("error comparing commits: {}", e))?
-    .ok_or_else(|| format!("could not find end commit for bound {:?}", end))?;
+    let comparison =
+        compare_given_commits(body.start, end.clone(), body.stat, ctxt, &master_commits)
+            .await
+            .map_err(|e| format!("error comparing commits: {}", e))?
+            .ok_or_else(|| format!("could not find end commit for bound {:?}", end))?;
 
     let conn = ctxt.conn().await;
     let prev = comparison.prev(&master_commits);
@@ -353,7 +346,7 @@ pub async fn compare(
     ctxt: &SiteCtxt,
 ) -> Result<Option<Comparison>, BoxedError> {
     let master_commits = collector::master_commits().await?;
-    compare_given_commits(start, end, stat, ctxt, &master_commits, false).await
+    compare_given_commits(start, end, stat, ctxt, &master_commits).await
 }
 
 /// Compare two bounds on a given stat
@@ -363,7 +356,6 @@ async fn compare_given_commits(
     stat: String,
     ctxt: &SiteCtxt,
     master_commits: &[collector::MasterCommit],
-    calc_new_sig: bool,
 ) -> Result<Option<Comparison>, BoxedError> {
     let a = ctxt
         .artifact_id_for_bound(start.clone(), true)
@@ -401,7 +393,6 @@ async fn compare_given_commits(
                     scenario: test_case.2,
                     variance: variances.data.get(&test_case).cloned(),
                     results: (a, b),
-                    calc_new_sig,
                 })
         })
         .collect();
@@ -751,17 +742,10 @@ impl BenchmarkVariance {
     }
 
     /// Whether we can trust this benchmark or not
-    fn is_dodgy(&self, calc_new_sig: bool) -> bool {
-        if !calc_new_sig {
-            matches!(
-                self.description,
-                BenchmarkVarianceDescription::Noisy | BenchmarkVarianceDescription::HighlyVariable
-            )
-        } else {
-            // If changes are judged significant only exceeding 0.2%, then the
-            // benchmark as a whole is dodgy.
-            self.significance_threshold() * 100.0 > 0.2
-        }
+    fn is_dodgy(&self) -> bool {
+        // If changes are judged significant only exceeding 0.2%, then the
+        // benchmark as a whole is dodgy.
+        self.significance_threshold() * 100.0 > 0.2
     }
 }
 
@@ -816,18 +800,13 @@ pub struct TestResultComparison {
     scenario: Scenario,
     variance: Option<BenchmarkVariance>,
     results: (f64, f64),
-    calc_new_sig: bool,
 }
 
 impl TestResultComparison {
     /// The amount of relative change considered significant when
     /// we cannot determine from historical data
     const SIGNIFICANT_RELATIVE_CHANGE_THRESHOLD: f64 = 0.002;
 
-    /// The amount of relative change considered significant when
-    /// the test case is dodgy
-    const SIGNIFICANT_RELATIVE_CHANGE_THRESHOLD_DODGY: f64 = 0.008;
-
     fn is_regression(&self) -> bool {
         let (a, b) = self.results;
         b > a
@@ -843,18 +822,10 @@ impl TestResultComparison {
 
     // Magnitude of change considered significant
     fn significance_threshold(&self) -> f64 {
-        if !self.calc_new_sig {
-            if self.is_dodgy() {
-                Self::SIGNIFICANT_RELATIVE_CHANGE_THRESHOLD_DODGY
-            } else {
-                Self::SIGNIFICANT_RELATIVE_CHANGE_THRESHOLD
-            }
-        } else {
-            self.variance
-                .as_ref()
-                .map(|v| v.significance_threshold())
-                .unwrap_or(Self::SIGNIFICANT_RELATIVE_CHANGE_THRESHOLD)
-        }
+        self.variance
+            .as_ref()
+            .map(|v| v.significance_threshold())
+            .unwrap_or(Self::SIGNIFICANT_RELATIVE_CHANGE_THRESHOLD)
     }
 
     /// This is a numeric magnitude of a particular change.
@@ -879,9 +850,6 @@ impl TestResultComparison {
         } else {
             Magnitude::VeryLarge
         };
-        if !self.calc_new_sig {
-            return over_threshold;
-        }
         let change_magnitude = if change < 0.002 {
             Magnitude::VerySmall
         } else if change < 0.01 {
@@ -918,7 +886,7 @@ impl TestResultComparison {
     fn is_dodgy(&self) -> bool {
         self.variance
             .as_ref()
-            .map(|v| v.is_dodgy(self.calc_new_sig))
+            .map(|v| v.is_dodgy())
             .unwrap_or(false)
     }
 

site/static/compare.html

@@ -827,7 +827,6 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
                 end: "",
                 stat: "instructions:u",
             }, state);
-            values["calcNewSig"] = values.calcNewSig === 'true';
             makeRequest("/get", values).then(function (data) {
                 app.data = data;
             });
@@ -849,4 +848,4 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
     </script>
 </body>
 
-</html>
+</html>