Skip to content

Commit

Permalink
Deconfuse Geomean Metric and geomean aggregate value (#408)
Browse files Browse the repository at this point in the history
The metric's "geomean" aggregate value can be easily confused with the top-level Geomean metric (issue #407).

- Don't serialize "geomean" in the JSON data
- Add descriptions to Score, Geomean, and Iteration metrics that are serialized in the JSON data
- Make the detail view titles more descriptive
  • Loading branch information
camillobruni committed May 30, 2024
1 parent 3ae822b commit 33dec4f
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 8 deletions.
6 changes: 3 additions & 3 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ <h1>Score</h1>
<h1 class="section-header">Detailed Results</h1>
<div class="section-content all-metric-results">
<div class="aggregated-metric-result">
<h2>Geomean</h2>
<h2>Aggregate Metric</h2>
<div id="geomean-chart"></div>
<h2>Tests</h2>
<h2>Test Metrics Overview</h2>
<div id="tests-chart"></div>
</div>
<br />
<h2>Detailed Metrics</h2>
<h2>Test Metrics Details</h2>
<div id="metrics-results"></div>
</div>
<div class="buttons section-footer">
Expand Down
6 changes: 3 additions & 3 deletions resources/benchmark-runner.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -574,9 +574,9 @@ export class BenchmarkRunner {
// Prepare all iteration metrics so they are listed at the end of
// of the _metrics object, before "Total" and "Score".
for (let i = 0; i < this._iterationCount; i++)
iterationTotalMetric(i);
getMetric("Geomean");
getMetric("Score", "score");
iterationTotalMetric(i).description = `Test totals for iteration ${i}`;
getMetric("Geomean", "ms").description = "Geomean of test totals";
getMetric("Score", "score").description = "Scaled inverse of the Geomean";
}

const geomean = getMetric("Geomean");
Expand Down
13 changes: 11 additions & 2 deletions resources/metric.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,15 @@ export class Metric {
throw new Error(`Invalid metric.name=${name}, expected string.`);
this.name = name;
this.unit = unit;
this.description = "";

this.mean = 0.0;
this.geomean = 0.0;
// Make "geomean" non-enumerable so we don't serialize it with JSON.stringify
// and avoid some confusion with the top-level Geomean metric.
Object.defineProperty(this, "geomean", {
writable: true,
value: 0,
});
this.delta = 0.0;
this.percentDelta = 0.0;

Expand All @@ -21,7 +27,6 @@ export class Metric {
this.max = 0.0;

this.values = [];
this.children = [];

// Mark properties which refer to other Metric objects as
// non-enumerable to avoid issue with JSON.stringify due to circular
Expand All @@ -31,6 +36,10 @@ export class Metric {
writable: true,
value: undefined,
},
children: {
writable: true,
value: [],
},
});
}

Expand Down

0 comments on commit 33dec4f

Please sign in to comment.