Skip to content

Commit

Permalink
Fix measurement proccessing, distinguish by runId, count properly, an…
Browse files Browse the repository at this point in the history
…d make sure we have all dimensions

- add tests that would have caught this
  • Loading branch information
smarr committed Jun 11, 2023
1 parent 78171f5 commit 5198588
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 42 deletions.
1 change: 1 addition & 0 deletions src/db-data-processing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ function findMeasurements(
if (
mm.envId == row.envid &&
mm.commitId == row.commitid &&
mm.runId == row.runid &&
mm.criterion.name == row.criterion
) {
m = mm;
Expand Down
70 changes: 35 additions & 35 deletions src/stats-data-prep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ function assertBasicPropertiesOfSortedMeasurements(

function addOrGetCompareStatsRow(
variants: Map<string, CompareStatsRow>,
countsAndMissing: VariantCountAndMissing | null,
measurements: Measurements,
bench: ProcessedResult
): CompareStatsRow {
Expand All @@ -196,17 +197,17 @@ function addOrGetCompareStatsRow(
let row = variants.get(key);
if (row === undefined) {
row = {
benchId: { b, e, s, v, c, ea },
benchId: { b, e, s, v, c, i, ea },
details: {
cmdline: measurements.runSettings.simplifiedCmdline,
envId,
hasProfiles: false,
hasWarmup: false,
numV: 0,
numC: 0,
numI: 0,
numEa: 0,
numEnv: 0
numV: countsAndMissing === null ? 0 : countsAndMissing.numV,
numC: countsAndMissing === null ? 0 : countsAndMissing.numC,
numI: countsAndMissing === null ? 0 : countsAndMissing.numI,
numEa: countsAndMissing === null ? 0 : countsAndMissing.numEa,
numEnv: countsAndMissing === null ? 0 : countsAndMissing.numEnv
}
};
variants.set(key, row);
Expand All @@ -221,7 +222,7 @@ function addMissingCompareStatsRow(
base: string,
change: string
): void {
const row = addOrGetCompareStatsRow(variants, measurements, bench);
const row = addOrGetCompareStatsRow(variants, null, measurements, bench);

if (!row.missing) {
row.missing = [];
Expand Down Expand Up @@ -263,7 +264,7 @@ export function countVariantsAndDropMissing(
let numC = 0;
let numI = 0;
let numEa = 0;
let numEnv = 1;
let numEnv = 0;

function dropAsMissing(i: number): void {
addMissingCompareStatsRow(missing, measurements[i], bench, base, change);
Expand All @@ -290,37 +291,29 @@ export function countVariantsAndDropMissing(

const base = measurements[i];

if (lastEnvId === -1) {
if (lastEnvId !== base.envId) {
lastEnvId = base.envId;
lastVarValue = base.runSettings.varValue;
lastCores = base.runSettings.cores;
lastInputSize = base.runSettings.inputSize;
lastExtraArgs = base.runSettings.extraArgs;
} else {
if (lastEnvId !== base.envId) {
lastEnvId = base.envId;
numEnv += 1;
}
numEnv += 1;
}

if (lastVarValue !== base.runSettings.varValue) {
lastVarValue = base.runSettings.varValue;
numV += 1;
}
if (lastVarValue !== base.runSettings.varValue) {
lastVarValue = base.runSettings.varValue;
numV += 1;
}

if (lastCores !== base.runSettings.cores) {
lastCores = base.runSettings.cores;
numC += 1;
}
if (lastCores !== base.runSettings.cores) {
lastCores = base.runSettings.cores;
numC += 1;
}

if (lastInputSize !== base.runSettings.inputSize) {
lastInputSize = base.runSettings.inputSize;
numI += 1;
}
if (lastInputSize !== base.runSettings.inputSize) {
lastInputSize = base.runSettings.inputSize;
numI += 1;
}

if (lastExtraArgs !== base.runSettings.extraArgs) {
lastExtraArgs = base.runSettings.extraArgs;
numEa += 1;
}
if (lastExtraArgs !== base.runSettings.extraArgs) {
lastExtraArgs = base.runSettings.extraArgs;
numEa += 1;
}

if (isLastItem) {
Expand Down Expand Up @@ -387,6 +380,7 @@ export async function calculateChangeStatsForBenchmark(
// - calculate the change statistics for each criterion
const counts = await computeStatisticsAndInlinePlot(
variants,
countsAndMissing,
bench,
measurements,
baseOffset,
Expand Down Expand Up @@ -443,6 +437,7 @@ export function getMsFlattenedAndSorted(

async function computeStatisticsAndInlinePlot(
variants: Map<string, CompareStatsRow>,
countsAndMissing: VariantCountAndMissing,
bench: ProcessedResult,
measurements: Measurements[],
baseOffset: number,
Expand All @@ -462,7 +457,12 @@ async function computeStatisticsAndInlinePlot(
const { sortedBase, sortedChange } = getMsFlattenedAndSorted(base, change);
const changeStats = calculateChangeStatistics(sortedBase, sortedChange);

const row = addOrGetCompareStatsRow(variants, change, bench);
const row = addOrGetCompareStatsRow(
variants,
countsAndMissing,
change,
bench
);

// add the various details
row.details.hasWarmup = siteConfig.canShowWarmup(change.values);
Expand Down
12 changes: 6 additions & 6 deletions src/views/compare/stats-row.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@

let args = '';

if (stats.details.numV > 0) { args += stats.benchId.v + ' '; }
if (stats.details.numC > 0) { args += stats.benchId.c + ' '; }
if (stats.details.numI > 0) { args += stats.benchId.i + ' '; }
if (stats.details.numEa > 0) { args += stats.benchId.ea + ' '; }
if (stats.details.numV > 1) { args += stats.benchId.v + ' '; }
if (stats.details.numC > 1) { args += stats.benchId.c + ' '; }
if (stats.details.numI > 1) { args += stats.benchId.i + ' '; }
if (stats.details.numEa > 1) { args += stats.benchId.ea + ' '; }

if (args.length > 0) {
args = `<span class="all-args">${args.trim()}</span>`;
}

if (stats.missing) {
%}<tr>
%}<th scope="row">{%= stats.benchId.b %}{%= args %}</th>
%}<th scope="row">{%= stats.benchId.b %}{%- args %}</th>
<td colspan="4">Missing data for {%
const byCommitId = new Map();
for (const m of stats.missing) {
Expand Down Expand Up @@ -46,7 +46,7 @@

if (hasTotal) {
%}<tr>
<th scope="row">{%= stats.benchId.b %}{%= args %}</th>
<th scope="row">{%= stats.benchId.b %}{%- args %}</th>
<td class="inline-cmp"><img src="{%= it.config.reportsUrl %}/{%= stats.inlinePlot %}"></td>
{% if (stats.exeStats) {
%}{%- include('compare/stats-row-across-exes.html', {
Expand Down
73 changes: 72 additions & 1 deletion tests/db-data-processing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
ResultsByExeSuiteBenchmark,
ResultsBySuiteBenchmark
} from '../src/stats-data-prep.js';
import { ProcessedResult } from '../src/db.js';
import { MeasurementData, ProcessedResult } from '../src/db.js';

const dataJsSOM = JSON.parse(
readFileSync(
Expand Down Expand Up @@ -175,4 +175,75 @@ describe('collateMeasurements()', () => {
expect(m2.values[0]).toHaveLength(120);
});
});

describe('needs to distinguish different run ids', () => {
function createMeasure(
runid: number,
inputsize: string,
commitid: string
): MeasurementData {
return {
expid: 1,
runid,
trialid: 1,
commitid,

bench: 'b',
exe: 'e',
suite: 's',

cmdline: 'b e s ' + inputsize,
varvalue: null,
cores: null,
inputsize,
extraargs: null,

iteration: 1,
invocation: 1,
warmup: null,

criterion: 'total',
unit: 'ms',
value: 1,

envid: 1
};
}

const data: MeasurementData[] = [
createMeasure(1, '1', 'a'),
createMeasure(1, '1', 'b'),
createMeasure(2, '2', 'a'),
createMeasure(2, '2', 'b')
];

const result: ResultsByExeSuiteBenchmark = collateMeasurements(data);

it('should have 1 executor', () => {
expect(result.size).toBe(1);
expect([...result.keys()]).toEqual(['e']);
});

it('should have 1 suite', () => {
const suites = <ResultsBySuiteBenchmark>result.get('e');
expect(suites.size).toBe(1);
expect([...suites.keys()]).toEqual(['s']);
});

it('should have the expected benchmark', () => {
const suites = <ResultsBySuiteBenchmark>result.get('e');
const bs = <ResultsByBenchmark>suites.get('s');
expect(bs.benchmarks.size).toBe(1);

expect([...bs.benchmarks.keys()]).toEqual(['b']);
});

it('should have 4 measurements', () => {
const suites = <ResultsBySuiteBenchmark>result.get('e');
const bs = <ResultsByBenchmark>suites.get('s');
const b = <ProcessedResult>bs.benchmarks.get('b');

expect(b.measurements).toHaveLength(4);
});
});
});
10 changes: 10 additions & 0 deletions tests/stats-data-prep.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,16 @@ describe('calculateChangeStatsForBenchmark()', () => {
it('should not have dropped any data', () => {
expect(nbody.measurements).toHaveLength(2);
expect(stats.stats).toHaveLength(1);

for (const propName of ['b', 'e', 's', 'v', 'c', 'i', 'ea']) {
expect(stats.stats[0].benchId).toHaveProperty(propName);
}

expect(stats.stats[0].details.numV).toBe(0);
expect(stats.stats[0].details.numC).toBe(1);
expect(stats.stats[0].details.numI).toBe(0);
expect(stats.stats[0].details.numEa).toBe(1);
expect(stats.stats[0].details.numEnv).toBe(1);
});

it('should have added the expected statistics', () => {
Expand Down

0 comments on commit 5198588

Please sign in to comment.