Add wasm-only and webgpu-only scores#171
Conversation
5b1f65f to
8829673
Compare
b514574 to
ada6b1f
Compare
|
|
||
| function geomeanToScore(geomean) { | ||
| return 1000 / geomean; | ||
| return 10000 / geomean; |
There was a problem hiding this comment.
Given that we are changing the score in a major way, do we want to bump the WebAI version to 0.3.0?
| iterationMetric(i, "WASM-Total").description = `WASM test totals for iteration ${i}`; | ||
| iterationMetric(i, "WebGPU-Total").description = `WebGPU test totals for iteration ${i}`; | ||
| } | ||
| getMetric("WASM-Geomean", "ms").description = "Geomean of WASM test totals"; |
There was a problem hiding this comment.
I know it's a small thing, but it's important to many WebAssembly folks: It's Wasm not WASM (since it's not an acronym). Please change that everywhere (in both the metric names and text displayed on the results page etc.)
| const score = 1000 / geomean; | ||
| const score = 10000 / geomean; | ||
|
|
||
| const { total: measuredTotal, mean: measuredMean, geomean: measuredGeomean, score: measuredScore } = runner._measuredValues; |
There was a problem hiding this comment.
Is it intentional that we are removing the *Total and *Mean from _measuredValues? IIUC, we now only have *Geomean and *Score each?
| const mean = total / suite.steps.length; | ||
| const geomean = Math.pow(total, 1 / suite.steps.length); | ||
| const score = 1000 / geomean; | ||
| const score = 10000 / geomean; |
There was a problem hiding this comment.
nit: Can we use geomeanToScore here instead, so we avoid an unnamed constant (which could be forgotten to be updated alongside the actual score calculation above)
| geomean = Math.pow(product, 1 / values.length); | ||
| mean = total / values.length; | ||
| } | ||
| const wasmGeomean = !isNaN(iterationWasmMetric?.geomean) ? iterationWasmMetric.geomean : 0; |
There was a problem hiding this comment.
Instead of silently producing a 0 score, could we instead assert that there are no NaN results?
|
|
||
| let mean = 0; | ||
| if (values.length > 0) { | ||
| values.sort((a, b) => a - b); // Avoid the loss of significance for the sum. |
There was a problem hiding this comment.
Mhm, I didn't have a look at this before, but why were we calculating another geomean here?
| iterationWasmTotal.add(results.total); | ||
| } else if (suite?.tags?.includes("webgpu")) { | ||
| iterationWebgpuTotal.add(results.total); | ||
| } |
There was a problem hiding this comment.
can we add an else { assert(false); } here to make sure there are not unaccounted-for suites that are neither Wasm or WebGPU?
This pull request updated the benchmark to tracking and displaying two independent performance metrics: WASM Score and WebGPU Score. Additionally, the score calculation scaling factor has been adjusted from
100,000 / geomeanto10,000 / geomeanto produce more intuitive, readable score numbers across all workloads. The UI and test suites have also been updated to support these dual scores and ensure stable end-to-end reporting when running subset benchmarks.Closes #170