Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(262)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: tracing/tracing/metrics/compare_samples.html

Issue 2089833002: Entry point for bisect sample comparison. (Closed) Base URL: https://github.com/catapult-project/catapult.git@mann
Patch Set: A couple more changes. Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« tracing/bin/compare_samples ('K') | « tracing/bin/compare_samples ('k') | tracing/tracing/metrics/compare_samples.py » ('j') | tracing/tracing/metrics/compare_samples.py » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tracing/tracing/metrics/compare_samples.html
diff --git a/tracing/tracing/metrics/compare_samples.html b/tracing/tracing/metrics/compare_samples.html
new file mode 100644
index 0000000000000000000000000000000000000000..135be9c209bc744fc08a5d3af44b545517a1ff9c
--- /dev/null
+++ b/tracing/tracing/metrics/compare_samples.html
@@ -0,0 +1,248 @@
+<!DOCTYPE html>
+<!--
+Copyright 2016 The Chromium Authors. All rights reserved.
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+-->
+
+<link rel="import" href="/tracing/base/statistics.html">
+<link rel="import" href="/tracing/base/xhr.html">
+<link rel="import" href="/tracing/value/value_set.html">
+
+<script>
+'use strict';
+
+tr.exportTo('tr.metrics', function() {
+ var BisectComparison = {
+ ENOUGH_SAMPLES: 18,
+ SIGNIFICANCE_LEVEL: 0.05,
benjhayden 2016/08/10 17:40:21 Can these constants go in tr.v.Numeric so they can
RobertoCN 2016/09/02 21:19:31 These constants should really be specific to bisec
benjhayden 2016/09/02 23:04:40 Why should results2.html and the metric side panel
RobertoCN 2016/09/03 00:21:59 Over time, improving the rate of False Positives/I
benjhayden 2016/09/07 16:20:58 IIUC, the expectation is not that metric-side-pane
+ escapeChars: s => s.replace(/[\:|=\/#&,]/g, '_'),
benjhayden 2016/08/10 17:40:21 Can these helper methods be moved outside of Bisec
RobertoCN 2016/09/02 21:19:30 Done.
+ valuesFromCharts: function(listOfCharts, metricName) {
+ function geoMeanFromHistogram(h) {
benjhayden 2016/08/10 17:40:21 Can you move this to Numeric? Or, if |h| is not a
benjhayden 2016/08/18 22:06:34 Can you find a streaming algorithm and integrate i
RobertoCN 2016/09/02 21:19:30 Filed a bug to do this separately. (See below)
RobertoCN 2016/09/02 21:19:31 Filed a bug to this later: https://github.com/cata
+ if (!h.hasOwnProperty('buckets')) {
+ return 0.0;
+ }
+ var count = 0;
+ var sumOfLogs = 0;
+ for (var bucket of h.buckets) {
+ if (bucket.hasOwnProperty('high')) {
+ bucket.mean = (bucket.low + bucket.high) / 2.0;
+ } else {
+ bucket.mean = bucket.low;
+ }
+
+ if (bucket.mean > 0) {
+ sumOfLogs += Math.log(bucket.mean) * bucket.count;
+ count += bucket.count;
+ }
+ }
+ if (count === 0) {
+ return 0.0;
+ }
+ return Math.exp(sumOfLogs / count);
+ }
+ var all_values = [];
+ for (var charts of listOfCharts) {
+ var chartName, interactionName, traceName;
benjhayden 2016/08/10 17:40:21 Can these vars go on separate lines so they're eas
RobertoCN 2016/09/02 21:19:30 Done.
+ var parts = metricName.split('/');
+ if (parts.length === 3) {
+ chartName = parts[0];
+ interactionName = parts[1];
+ traceName = parts[2];
+ } else if (parts.length === 2) {
+ chartName = parts[0];
+ if (chartName != parts[1]) {
+ traceName = parts[1];
+ }
+ } else {
+ throw new Error('Could not parse metric name.');
+ }
+
+ if (interactionName) {
+ chartName = interactionName + '@@' + chartName;
+ }
benjhayden 2016/08/10 17:40:21 No braces necessary for single-line blocks.
RobertoCN 2016/09/02 21:19:31 Done.
+
+ if (!traceName) {
+ traceName = 'summary';
+ }
+
+ var chart, trace;
+ for (chart in charts.charts) {
benjhayden 2016/08/18 22:06:34 This should use tr.b.iterDictionaryKeys() so you d
RobertoCN 2016/09/02 21:19:31 Done.
+ if (charts.charts.hasOwnProperty(chart) &&
+ this.escapeChars(chart) === chartName) {
+ chartName = chart; // Unescaping
+ break;
+ }
+ }
+ for (trace in charts.charts[chartName]) {
+ if (charts.charts[chartName].hasOwnProperty(trace) &&
benjhayden 2016/08/18 22:06:34 Can you refactor these similar loops?
RobertoCN 2016/09/02 21:19:31 Done.
+ this.escapeChars(trace) === traceName) {
+ traceName = trace; // Unescaping
+ break;
+ }
+ }
+ if (charts.charts[chartName][traceName].type ==
+ 'list_of_scalar_values') {
+ all_values.push.apply(
+ all_values, charts.charts[chartName][traceName].values);
benjhayden 2016/08/18 22:06:34 all_values.push(...charts.charts[chartName][traceN
RobertoCN 2016/09/02 21:19:31 Done.
+ }
+
+ if (charts.charts[chartName][traceName].type === 'histogram') {
+ all_values.push(
+ geoMeanFromHistogam(charts.charts[chartName][traceName]));
+ }
+ }
+ return all_values;
+ },
+
+ compareValuesets: function(valueSetAPathList, valueSetBPathList, metric) {
+ var aPaths = valueSetAPathList.split(',');
+ var bPaths = valueSetBPathList.split(',');
+ var valueSetA = new tr.v.ValueSet();
+ var valueSetB = new tr.v.ValueSet();
+ var current;
+ for (var path of aPaths) {
+ try {
+ current = tr.b.getSync('file://' + path);
+ } catch (ex) {
+ var err = new Error('Could not open' + path);
+ err.name = 'File loading error';
+ throw err;
+ }
+ valueSetA.addValuesFromDicts(JSON.parse(current));
+ }
+ for (var path of bPaths) {
benjhayden 2016/08/10 17:40:21 Can this be refactored to reduce code duplication?
RobertoCN 2016/09/02 21:19:31 Done.
+ try {
+ current = tr.b.getSync('file://' + path);
+ } catch (ex) {
+ var err = new Error('Could not open' + path);
+ err.name = 'File loading error';
+ throw err;
+ }
+ valueSetB.addValuesFromDicts(JSON.parse(current));
+ }
+
+ function rawValuesByMetricName(valueSet, metricName, thisParam) {
benjhayden 2016/08/10 17:40:21 Can this function be moved outside of BisectCompar
RobertoCN 2016/09/02 21:19:30 Done.
+ var interactionRecord, valueName, story;
+ var metricNameParts = metricName.split('/');
+ if (metricNameParts[0] === metricNameParts[1]) {
+ story = 'summary';
+ } else {
+ story = metricNameParts[1];
+ }
+ var chartNameParts = metricNameParts[0].split('-');
+ valueName = chartNameParts[1];
+ if (chartNameParts.length === 2) {
+ interactionRecord = chartNameParts[0];
+ }
+ var values = valueSet.getValuesWithName(valueName);
+ if (!values || values.length === 0) {
+ // If there was a dash in the chart name, but it wasn't an
+ // interaction record.
+ valueName = metricNameParts[0];
+ values = valueSet.getValuesWithName(valueName);
+ interactionRecord = undefined;
+ if (!values || values.length === 0) {
+ throw new Error('No values with name ' + valueName);
+ }
+ }
+ var filtered = values.filter(function(value) {
+ if (value.name != valueName) {
benjhayden 2016/08/18 22:14:11 var filtered = []; for (var value of values) { i
RobertoCN 2016/09/02 21:19:30 Done.
+ return false;
+ }
+ var ii = tr.v.d.IterationInfo.getFromValue(value);
+ if (interactionRecord) {
+ var values = [];
+ var keys = Object.keys(ii.storyGroupingKeys);
+ keys.sort();
+ for (var key of keys) {
+ values.push(ii.storyGroupingKeys[key]);
+ }
+ if (interactionRecord === values.join('_')) {
+ return thisParam.escapeChars(ii.storyDisplayName) ==
+ thisParam.escapeChars(story);
+ }
+ return false;
+ }
+ return thisParam.escapeChars(ii.storyDisplayName) ==
+ thisParam.escapeChars(story);
+ });
+ var rawValues = [];
+ for (var val of filtered) {
+ if (val.numeric instanceof tr.v.Numeric) {
+ rawValues = rawValues.concat(val.numeric.sampleValues);
+ } else if (val.numeric instanceof tr.v.ScalarNumeric) {
+ rawValues.push(val.numeric.value);
+ }
+ }
+ return rawValues;
+ }
+ var sampleA = rawValuesByMetricName(valueSetA, metric, this);
+ var sampleB = rawValuesByMetricName(valueSetB, metric, this);
+ return this.compareSamples(sampleA, sampleB);
+ },
+
+ compareCharts: function(chartPathListA, chartPathListB, metric) {
+ var aPaths = chartPathListA.split(',');
+ var bPaths = chartPathListB.split(',');
+ var chartsA = [];
+ var chartsB = [];
+ var current;
+ for (var path of aPaths) {
+ try {
+ current = tr.b.getSync('file://' + path);
+ } catch (ex) {
+ var err = new Error('Could not open' + path);
+ err.name = 'File loading error';
+ throw err;
+ }
+ chartsA.push(JSON.parse(current));
+ }
+ for (var path of bPaths) {
+ try {
+ current = tr.b.getSync('file://' + path);
+ } catch (ex) {
+ var err = new Error('Could not open' + path);
+ err.name = 'File loading error';
+ throw err;
+ }
+ chartsB.push(JSON.parse(current));
+ }
+ var sampleA = this.valuesFromCharts(chartsA, metric);
+ var sampleB = this.valuesFromCharts(chartsB, metric);
+ return this.compareSamples(sampleA, sampleB);
+ },
+
+ compareSamples: function(sampleA, sampleB) {
benjhayden 2016/08/10 17:40:21 Can this be refactored to share code with Numeric?
RobertoCN 2016/09/02 21:19:31 The difference is that these samples are flat list
+ var pValue = tr.b.Statistics.mwu.test(sampleA, sampleB);
+ // Diagnostics
benjhayden 2016/08/19 00:38:53 Please open a bug to remove this code when bisect
RobertoCN 2016/09/02 21:19:30 Acknowledged.
+ var result = {
+ sample_a: {
+ std_dev: tr.b.Statistics.stddev(sampleA),
+ mean: tr.b.Statistics.mean(sampleA),
+ debug_values: sampleA
+ },
+ sample_b: {
benjhayden 2016/08/19 00:38:53 Please deduplicate this code using a function.
RobertoCN 2016/09/02 21:19:30 Done.
+ std_dev: tr.b.Statistics.stddev(sampleB),
+ mean: tr.b.Statistics.mean(sampleB),
+ debug_values: sampleB
+ },
+ pValue: pValue.p,
+ UStatistic: pValue.U,
+ result: 'needMoreData',
+ };
+ if (pValue.p < this.SIGNIFICANCE_LEVEL) {
+ result.result = true; // Reject the null
+ } else if (sampleA.length > this.ENOUGH_SAMPLES &&
+ sampleB.length > this.ENOUGH_SAMPLES) {
+ result.result = false; // Fail to reject the null.
+ }
+ return result;
+ }
+ };
+
+ return {
+ BisectComparison: BisectComparison
+ };
+});
+</script>
« tracing/bin/compare_samples ('K') | « tracing/bin/compare_samples ('k') | tracing/tracing/metrics/compare_samples.py » ('j') | tracing/tracing/metrics/compare_samples.py » ('J')

Powered by Google App Engine
This is Rietveld 408576698