Rename horizon to auto sample conditions (#225)

I never liked the name "horizons" (except on the day I chose it I guess). I think a much clearer name is "auto sample conditions".

This changes the `--horizon` flag and the `horizons` JSON config setting (yes... one was singular, the other plural) to `--auto-sample-conditions` and `autoSampleConditions` respectively.

It's backwards compatible, with a warning printed if you use the old style.

Related to #221

Author: aomarks

CHANGELOG.md

@@ -7,6 +7,11 @@ project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Unreleased
 
+- `--horizons` flag and `horizon` config setting has been replaced with the
+  `--auto-sample-conditions` and `autoSampleConditions`. `--horizon` will
+  continue to work for backwards compatibility, but please do update to the new
+  name.
+
 - Copyright notice owner changed from "The Polymer Project Authors" to "Google
   LLC". Trivial reformatting for `LICENSE` file to match spdx.org version.
   Source license headers replaced with concise SPDX-style.

README.md

@@ -120,7 +120,7 @@ By default, a **minimum of 50 samples** are taken from **each** benchmark. You
 can change the minimum sample size with the `--sample-size` flag or the
 `sampleSize` JSON config option.
 
-### Auto sampling
+### Auto sample
 
 After the initial 50 samples, tachometer will continue taking samples until
 there is a clear statistically significant difference between all benchmarks,
@@ -130,33 +130,36 @@ You can change this duration with the `--timeout` flag or the `timeout` JSON
 config option, measured in minutes. Set `--timeout=0` to disable auto sampling
 entirely. Set `--timeout=60` to sample for up to an hour.
 
-### Horizons
+### Auto sample conditions
 
 You can also configure which statistical conditions tachometer should check for
-when deciding when to stop auto sampling by configuring _horizons_.
+when deciding when to stop auto sampling by configuring _auto sample
+conditions_.
 
-To set horizons from the command-line, use the `--horizon` flag with a
-comma-delimited list:
+To set auto sample conditions from the command-line, use the
+`--auto-sample-conditions` flag with a comma-delimited list:
 
 ```sh
---horizon=0%,10%
+--auto-sample-conditions=0%,10%
 ```
 
-To set horizons from a JSON config file, use the `horizons` property with an
-array of strings (including if there is only one condition):
+To set auto sample conditions from a JSON config file, use the
+`autoSampleConditions` property with an array of strings (including if there is
+only one condition):
 
 ```json
 {
-  "horizons": ["0%", "10%"]
+  "autoSampleConditions": ["0%", "10%"]
 }
 ```
 
-A horizon can be thought of as a point of interest on the number-line of either
-absolute milliseconds, or relative percent change. By setting a horizon, you are
-asking tachometer to try to shrink the confidence interval until it is
-unambiguously placed on one side or the other of that horizon.
+An auto sample condition can be thought of as a point of interest on the
+number-line of either absolute milliseconds, or relative percent change. By
+setting a condition, you are asking tachometer to try to shrink the confidence
+interval until it is unambiguously placed on one side or the other of that
+condition.
 
-| Example horizon     | Question                                                   |
+| Example condition   | Question                                                   |
 | ------------------- | ---------------------------------------------------------- |
 | `0%`                | Is A faster or slower than B _at all_? (The **default**)   |
 | `10%`               | Is A faster or slower than B by at least 10%?              |
@@ -166,24 +169,24 @@ unambiguously placed on one side or the other of that horizon.
 | `0%`, `10%`, `100%` | Is A at all, a little, or a lot slower or faster than B?   |
 | `0.5ms`             | Is A faster or slower than B by at least 0.5 milliseconds? |
 
-In the following example, we have set `--horizon=10%`, meaning we are interested
-in knowing whether A differs from B by at least 10% in either direction. The
-sample size automatically increases until the confidence interval is narrow
-enough to place the estimated difference squarely on one side or the other of
-both horizons.
+In the following example, we have set `--auto-sample-conditions=10%`, meaning we
+are interested in knowing whether A differs from B by at least 10% in either
+direction. The sample size automatically increases until the confidence interval
+is narrow enough to place the estimated difference squarely on one side or the
+other of both conditions.
 
 ```
-      <------------------------------->     n=50  ❌ -10% ❌ +10%
-                <------------------>        n=100 ✔️ -10% ❌ +10%
+      <------------------------------->     n=50  X -10% X +10%
+                <------------------>        n=100 ✔️ -10% X +10%
                     <----->                 n=200 ✔️ -10% ✔️ +10%
 
   |---------|---------|---------|---------| difference in runtime
 -20%      -10%        0       +10%      +20%
 
-n     = sample size
-<---> = confidence interval for percent difference of mean runtimes
-✔️    = resolved horizon
-❌    = unresolved horizon
+n    = sample size
+<--> = confidence interval for percent difference of mean runtimes
+✔️    = resolved condition
+X    = unresolved condition
 ```
 
 In this example, by `n=50` we are not sure whether A is faster or slower than B
@@ -192,10 +195,10 @@ than 10%, but we're still not sure if it's _slower_ by more than 10%. By `n=200`
 we have also ruled out that B is slower than A by more than 10%, so we stop
 sampling. Note that we still don't know which is _absolutely_ faster, we just
 know that whatever the difference is, it is neither faster nor slower than 10%
-(and if we did want to know, we could add `0` to our horizons).
+(and if we did want to know, we could add `0` to our conditions).
 
-Note that, if the _actual_ difference is very close to a horizon, then it is
-likely that the horizon will never be met, and the timeout will expire.
+Note that, if the _actual_ difference is very close to a condition, then it is
+likely that the condition will never be met, and the timeout will expire.
 
 ## Measurement modes
 
@@ -705,7 +708,7 @@ Defaults are the same as the corresponding command-line flags.
   "root": "./benchmarks",
   "sampleSize": 50,
   "timeout": 3,
-  "horizons": ["0%", "1%"],
+  "autoSampleConditions": ["0%", "1%"],
   "benchmarks": [
     {
       "name": "foo",
@@ -806,9 +809,9 @@ tach http://example.com
 | `--package-version` / `-p`  | _(none)_                                | Specify an NPM package version to swap in ([details](#swap-npm-dependencies))                                                                                      |
 | `--browser` / `-b`          | `chrome`                                | Which browsers to launch in automatic mode, comma-delimited (chrome, firefox, safari, edge, ie) ([details](#browsers))                                             |
 | `--window-size`             | `1024,768`                              | "width,height" in pixels of the browser windows that will be created                                                                                               |
-| `--sample-size` / `-n`      | `50`                                    | Minimum number of times to run each benchmark ([details](#sample-size)]                                                                                            |
-| `--horizon`                 | `0%`                                    | The degrees of difference to try and resolve when auto-sampling ("N%" or "Nms", comma-delimited) ([details](#auto-sampling))                                       |
-| `--timeout`                 | `3`                                     | The maximum number of minutes to spend auto-sampling ([details](#auto-sampling))                                                                                   |
+| `--sample-size` / `-n`      | `50`                                    | Minimum number of times to run each benchmark ([details](#sample-size))                                                                                            |
+| `--auto-sample-conditions`  | `0%`                                    | The degrees of difference to try and resolve when auto-sampling ("N%" or "Nms", comma-delimited) ([details](#auto-sample-conditions))                              |
+| `--timeout`                 | `3`                                     | The maximum number of minutes to spend auto-sampling ([details](#auto-sample))                                                                                     |
 | `--measure`                 | `callback`                              | Which time interval to measure (`callback`, `global`, `fcp`) ([details](#measurement-modes))                                                                       |
 | `--measurement-expression`  | `window.tachometerResult`               | JS expression to poll for on page to retrieve measurement result when `measure` setting is set to `global`                                                         |
 | `--remote-accessible-host`  | matches `--host`                        | When using a browser over a remote WebDriver connection, the URL that those browsers should use to access the local tachometer server ([details](#remote-control)) |

config.schema.json

@@ -453,6 +453,13 @@
             "description": "An optional reference to the JSON Schema for this file.\n\nIf none is given, and the file is a valid tachometer config file,\ntachometer will write back to the config file to give this a value.",
             "type": "string"
         },
+        "autoSampleConditions": {
+            "description": "The degrees of difference to try and resolve when auto-sampling\n(e.g. 0ms, +1ms, -1ms, 0%, +1%, -1%, default 0%).",
+            "items": {
+                "type": "string"
+            },
+            "type": "array"
+        },
         "benchmarks": {
             "description": "Benchmarks to run.",
             "items": {
@@ -462,7 +469,7 @@
             "type": "array"
         },
         "horizons": {
-            "description": "The degrees of difference to try and resolve when auto-sampling\n(e.g. 0ms, +1ms, -1ms, 0%, +1%, -1%, default 0%).",
+            "description": "Deprecated alias for autoSampleConditions.",
             "items": {
                 "type": "string"
             },

src/config.ts

@@ -13,7 +13,7 @@ import * as defaults from './defaults';
 import {Opts} from './flags';
 import {CheckConfig, parseGithubCheckFlag} from './github';
 import {specsFromOpts} from './specs';
-import {Horizons} from './stats';
+import {AutoSampleConditions} from './stats';
 import {BenchmarkSpec} from './types';
 import {fileKind} from './util';
 
@@ -25,7 +25,7 @@ export interface Config {
   sampleSize: number;
   timeout: number;
   benchmarks: BenchmarkSpec[];
-  horizons: Horizons;
+  autoSampleConditions: AutoSampleConditions;
   mode: 'automatic' | 'manual';
   jsonFile: string;
   // TODO(aomarks) Remove this in next major version.
@@ -69,8 +69,10 @@ export async function makeConfig(opts: Opts): Promise<Config> {
     if (opts.timeout !== undefined) {
       throw new Error('--timeout cannot be specified when using --config');
     }
-    if (opts.horizon !== undefined) {
-      throw new Error('--horizon cannot be specified when using --config');
+    if (opts['auto-sample-conditions'] !== undefined) {
+      throw new Error(
+        '--auto-sample-conditions cannot be specified when using --config'
+      );
     }
     if (opts.measure !== undefined) {
       throw new Error('--measure cannot be specified when using --config');
@@ -98,9 +100,9 @@ export async function makeConfig(opts: Opts): Promise<Config> {
       root: opts.root,
       sampleSize: opts['sample-size'],
       timeout: opts.timeout,
-      horizons:
-        opts.horizon !== undefined
-          ? parseHorizons(opts.horizon.split(','))
+      autoSampleConditions:
+        opts['auto-sample-conditions'] !== undefined
+          ? parseAutoSampleConditions(opts['auto-sample-conditions'].split(','))
           : undefined,
       benchmarks: await specsFromOpts(opts),
       resolveBareModules: opts['resolve-bare-modules'],
@@ -148,10 +150,10 @@ export function applyDefaults(partial: Partial<Config>): Config {
         ? partial.forceCleanNpmInstall
         : defaults.forceCleanNpmInstall,
     githubCheck: partial.githubCheck,
-    horizons:
-      partial.horizons !== undefined
-        ? partial.horizons
-        : parseHorizons([...defaults.horizons]),
+    autoSampleConditions:
+      partial.autoSampleConditions !== undefined
+        ? partial.autoSampleConditions
+        : parseAutoSampleConditions([...defaults.autoSampleConditions]),
     jsonFile: partial.jsonFile !== undefined ? partial.jsonFile : '',
     legacyJsonFile:
       partial.legacyJsonFile !== undefined ? partial.legacyJsonFile : '',
@@ -209,13 +211,17 @@ export async function urlFromLocalPath(
   return urlPath;
 }
 
-/** Parse horizon flags into signed horizon values. */
-export function parseHorizons(strs: string[]): Horizons {
+/**
+ * Parse auto sample condition strings.
+ */
+export function parseAutoSampleConditions(
+  strs: string[]
+): AutoSampleConditions {
   const absolute = new Set<number>();
   const relative = new Set<number>();
   for (const str of strs) {
     if (!str.match(/^[-+]?(\d*\.)?\d+(ms|%)$/)) {
-      throw new Error(`Invalid horizon ${str}`);
+      throw new Error(`Invalid auto sample condition ${str}`);
     }
 
     let num;
@@ -232,10 +238,10 @@ export function parseHorizons(strs: string[]): Horizons {
 
     if (str.startsWith('+') || str.startsWith('-') || num === 0) {
       // If the sign was explicit (e.g. "+0.1", "-0.1") then we're only
-      // interested in that signed horizon.
+      // interested in that signed condition.
       absOrRel.add(num);
     } else {
-      // Otherwise (e.g. "0.1") we're interested in the horizon as a
+      // Otherwise (e.g. "0.1") we're interested in the condition as a
       // difference in either direction.
       absOrRel.add(-num);
       absOrRel.add(num);

src/configfile.ts

@@ -15,7 +15,7 @@ import {
   parseBrowserConfigString,
   validateBrowserConfig,
 } from './browser';
-import {Config, parseHorizons, urlFromLocalPath} from './config';
+import {Config, parseAutoSampleConditions, urlFromLocalPath} from './config';
 import * as defaults from './defaults';
 import {makeUniqueSpecLabelFn} from './format';
 import {
@@ -53,6 +53,11 @@ export interface ConfigFile {
    * The degrees of difference to try and resolve when auto-sampling
    * (e.g. 0ms, +1ms, -1ms, 0%, +1%, -1%, default 0%).
    */
+  autoSampleConditions?: string[];
+
+  /**
+   * Deprecated alias for autoSampleConditions.
+   */
   horizons?: string[];
 
   /**
@@ -342,13 +347,26 @@ export async function parseConfigFile(
     }
   }
 
+  if (validated.horizons !== undefined) {
+    if (validated.autoSampleConditions !== undefined) {
+      throw new Error(
+        'Please use only "autoSampleConditions" and not "horizons".'
+      );
+    }
+    console.warn(
+      '\nNOTE: The "horizons" setting has been renamed to "autoSampleConditions".\n' +
+        'Please rename it.\n'
+    );
+    validated.autoSampleConditions = validated.horizons;
+  }
+
   return {
     root,
     sampleSize: validated.sampleSize,
     timeout: validated.timeout,
-    horizons:
-      validated.horizons !== undefined
-        ? parseHorizons(validated.horizons)
+    autoSampleConditions:
+      validated.autoSampleConditions !== undefined
+        ? parseAutoSampleConditions(validated.autoSampleConditions)
         : undefined,
     benchmarks,
     resolveBareModules: validated.resolveBareModules,

src/defaults.ts

@@ -15,7 +15,7 @@ export const browserName: BrowserName = 'chrome';
 export const headless = false;
 export const sampleSize = 50;
 export const timeout = 3;
-export const horizons = ['0%'] as const;
+export const autoSampleConditions = ['0%'] as const;
 export const mode = 'automatic';
 export const resolveBareModules = true;
 export const forceCleanNpmInstall = false;

src/flags.ts

@@ -170,11 +170,16 @@ export const optDefs: commandLineUsage.OptionDefinition[] = [
     defaultValue: defaults.measurementExpression,
   },
   {
-    name: 'horizon',
+    name: 'auto-sample-conditions',
     description:
       'The degrees of difference to try and resolve when auto-sampling ' +
       '(milliseconds, comma-delimited, optionally signed, ' +
-      `default ${defaults.horizons.join(',')})`,
+      `default ${defaults.autoSampleConditions.join(',')})`,
+    type: String,
+  },
+  {
+    name: 'horizon',
+    description: 'Deprecated alias for --auto-sample-conditions',
     type: String,
   },
   {
@@ -241,7 +246,7 @@ export interface Opts {
   save: string;
   measure: CommandLineMeasurements | undefined;
   'measurement-expression': string | undefined;
-  horizon: string | undefined;
+  'auto-sample-conditions': string | undefined;
   timeout: number | undefined;
   'github-check': string;
   'resolve-bare-modules': boolean | undefined;
@@ -265,6 +270,10 @@ export interface Opts {
   _unknown?: string[];
 }
 
+interface OptsWithDeprecated extends Opts {
+  horizon: string | undefined;
+}
+
 /**
  * Boolean flags that default to true are not supported
  * (https://github.com/75lb/command-line-args/issues/71).
@@ -286,7 +295,10 @@ function booleanString(flagName: string): (str: string) => boolean {
  * Parse the given CLI argument list.
  */
 export function parseFlags(argv: string[]): Opts {
-  const opts = commandLineArgs(optDefs, {partial: true, argv}) as Opts;
+  const opts = commandLineArgs(optDefs, {
+    partial: true,
+    argv,
+  }) as OptsWithDeprecated;
   // Note that when a flag is used but not set to a value (i.e. "tachometer
   // --resolve-bare-modules ..."), then the type function is not invoked, and
   // the value will be null. Since in default-false cases (which aren't
@@ -295,5 +307,18 @@ export function parseFlags(argv: string[]): Opts {
   if (opts['resolve-bare-modules'] === null) {
     opts['resolve-bare-modules'] = true;
   }
-  return opts;
+  if (opts['horizon']) {
+    if (opts['auto-sample-conditions']) {
+      throw new Error(
+        'Please use only --auto-sample-conditions and not --horizons.'
+      );
+    }
+    console.warn(
+      '\nNOTE: The --horizon flag has been renamed to --auto-sample-conditions.\n' +
+        'Please use --auto-sample-conditions going forward.\n'
+    );
+    opts['auto-sample-conditions'] = opts['horizon'];
+    delete opts['horizon'];
+  }
+  return opts as Opts;
 }