diff --git a/src/__tests__/CompareResults/__snapshots__/ResultsView.test.tsx.snap b/src/__tests__/CompareResults/__snapshots__/ResultsView.test.tsx.snap index ae8cf9fbc..9eca664b6 100644 --- a/src/__tests__/CompareResults/__snapshots__/ResultsView.test.tsx.snap +++ b/src/__tests__/CompareResults/__snapshots__/ResultsView.test.tsx.snap @@ -516,26 +516,6 @@ exports[`Results View Should display Base, New and Common graphs with replicates - - - Distribution - - - - - Estimated Modes - - - - - No modes or data for Base and New, possible oversmoothing, KDE evaluation failed - - @@ -1061,26 +1041,6 @@ exports[`Results View Should display Base, New and Common graphs with tooltips 1 - - - Distribution - - - - - Estimated Modes - - - - - No modes or data for Base and New, possible oversmoothing, KDE evaluation failed - - diff --git a/src/__tests__/helpers.test.ts b/src/__tests__/helpers.test.ts index 37edcde91..a95315c97 100644 --- a/src/__tests__/helpers.test.ts +++ b/src/__tests__/helpers.test.ts @@ -5,7 +5,6 @@ import { truncateHash, swapArrayElements, getDocsURL, - getModeInterpretation, capitalize, } from '../utils/helpers'; import getTestData from './utils/fixtures'; @@ -133,65 +132,6 @@ describe('swapArrayElements', () => { }); }); -describe('getModeInterpretation', () => { - const baseMode1 = 1; - const newMode1 = 1; - const baseMode2 = 2; - const newMode2 = 2; - const baseModenull = null; - const newMode0 = 0; - const baseMode0 = 0; - const newModenull = null; - - it('should handle same mode count interpretation for base and new', () => { - const expectedStr = getModeInterpretation(baseMode1, newMode1); - expect(expectedStr).toBe('Base and New revisions are unimodal'); - const expectedStr2 = getModeInterpretation(baseMode2, newMode2); - expect(expectedStr2).toBe('Base and New revisions are multimodal'); - }); - - it('should handle different mode count interpretation for base and new', () => { - const expectedStr2 = getModeInterpretation(baseMode2, newMode1); - expect(expectedStr2).toBe('Base is multimodal and New is unimodal'); - const expectedStr3 = getModeInterpretation(baseMode1, newMode2); - expect(expectedStr3).toBe('Base is unimodal and New is multimodal'); - }); - - it('should get mode interpretation with both baseRev and newRev as null or 0', () => { - const expectedStr4 = getModeInterpretation(baseModenull, newMode0); - expect(expectedStr4).toBe( - 'No modes or data for Base and New, possible oversmoothing, KDE evaluation failed', - ); - const expectedStr5 = getModeInterpretation(baseModenull, newModenull); - expect(expectedStr5).toBe( - 'No modes or data for Base and New, possible oversmoothing, KDE evaluation failed', - ); - const expectedStr6 = getModeInterpretation(baseMode0, newMode0); - expect(expectedStr6).toBe( - 'No modes or data for Base and New, possible oversmoothing, KDE evaluation failed', - ); - - const expectedStr7 = getModeInterpretation(baseMode0, newModenull); - expect(expectedStr7).toBe( - 'No modes or data for Base and New, possible oversmoothing, KDE evaluation failed', - ); - }); - - it('should get N/A for interpretModeCount one of baseRev or newRev < 1 and not null', () => { - const expectedStr1 = getModeInterpretation(baseMode2, newMode0); - expect(expectedStr1).toBe('Base is multimodal and New is N/A'); - const expectedStr2 = getModeInterpretation(baseMode0, newMode2); - expect(expectedStr2).toBe('Base is N/A and New is multimodal'); - }); - - it('should get N/A for interpretModeCount one of baseRev or newRev as null', () => { - const expectedStr0 = getModeInterpretation(baseModenull, newMode1); - expect(expectedStr0).toBe('Base is N/A and New is unimodal'); - const expectedStr3 = getModeInterpretation(baseMode1, newModenull); - expect(expectedStr3).toBe('Base is unimodal and New is N/A'); - }); -}); - describe('capitalize', () => { const string1 = 'i love my dog.'; const string2 = 'monkey'; diff --git a/src/__tests__/hooks/useSubtestRegressionCount.test.ts b/src/__tests__/hooks/useSubtestRegressionCount.test.ts index 4e1ee9a9f..471a3bdc0 100644 --- a/src/__tests__/hooks/useSubtestRegressionCount.test.ts +++ b/src/__tests__/hooks/useSubtestRegressionCount.test.ts @@ -104,7 +104,6 @@ describe('useSubtestRegressionCount', () => { newParentSignature: '200', replicates: false, testVersion: 'mann-whitney-u', - silvermanKDEEnabled: false, }); }); }); @@ -245,30 +244,6 @@ describe('useSubtestRegressionCount', () => { expect(hookResult.current.counts).not.toBeNull(); }); - it('passes silvermanKDEEnabled as true when the URL param is present', async () => { - window.history.replaceState( - null, - '', - '/compare-results?baseRev=abc&baseRepo=mozilla-central&framework=1&enable_silverman_kde', - ); - mockedFetchCompare.mockResolvedValue([]); - - renderHook(() => - useSubtestRegressionCount({ - result: baseResult, - view: compareView, - replicates: false, - testVersion: 'mann-whitney-u', - }), - ); - - await waitFor(() => { - expect(mockedFetchCompare).toHaveBeenCalledWith( - expect.objectContaining({ silvermanKDEEnabled: true }), - ); - }); - }); - it('calls memoizedFetchSubtestsCompareOverTimeResults with interval from URL for compareOverTimeView', async () => { window.history.replaceState( null, @@ -290,7 +265,6 @@ describe('useSubtestRegressionCount', () => { expect(mockedFetchCompareOverTime).toHaveBeenCalledWith( expect.objectContaining({ interval: 86400, - silvermanKDEEnabled: false, }), ); }); diff --git a/src/__tests__/utils/fixtures.ts b/src/__tests__/utils/fixtures.ts index 07a083d82..1572ade36 100644 --- a/src/__tests__/utils/fixtures.ts +++ b/src/__tests__/utils/fixtures.ts @@ -449,36 +449,8 @@ const getTestData = () => { effect_size: '', cles_explanation: '', }, - kde_new: { - median: 0, - sample_count: 0, - kde_x: [], - kde_y: [], - }, - kde_base: { - median: 0, - sample_count: 0, - kde_x: [], - kde_y: [], - }, - silverman_kde: { - bandwidth: '', - base_mode_count: 0, - new_mode_count: 0, - modes: [], - base_locations: [], - new_locations: [], - base_prominence: 0, - new_prominence: 0, - warnings: [], - is_regression: null, - is_improvement: null, - }, - is_fit_good: null, - is_significant: null, is_new_better: null, - performance_intepretation: '', direction_of_change: 'improvement', kde_warnings: [], }, @@ -568,35 +540,7 @@ const getTestData = () => { effect_size: '', cles_explanation: '', }, - kde_new: { - median: 0, - sample_count: 0, - kde_x: [], - kde_y: [], - }, - kde_base: { - median: 0, - sample_count: 0, - kde_x: [], - kde_y: [], - }, - silverman_kde: { - bandwidth: '', - base_mode_count: 0, - new_mode_count: 0, - modes: [], - base_locations: [], - new_locations: [], - base_prominence: 0, - new_prominence: 0, - warnings: [], - is_regression: null, - is_improvement: null, - }, - is_fit_good: null, - is_significant: null, is_new_better: null, - performance_intepretation: '', direction_of_change: 'regression', shapiro_wilk_test_base: null, kde_warnings: [], @@ -681,35 +625,7 @@ const getTestData = () => { effect_size: '', cles_explanation: '', }, - kde_new: { - median: 0, - sample_count: 0, - kde_x: [], - kde_y: [], - }, - kde_base: { - median: 0, - sample_count: 0, - kde_x: [], - kde_y: [], - }, - silverman_kde: { - bandwidth: '', - base_mode_count: 0, - new_mode_count: 0, - modes: [], - base_locations: [], - new_locations: [], - base_prominence: 0, - new_prominence: 0, - warnings: [], - is_regression: null, - is_improvement: null, - }, - is_fit_good: null, - is_significant: null, is_new_better: null, - performance_intepretation: '', direction_of_change: null, kde_warnings: [], }, @@ -792,35 +708,7 @@ const getTestData = () => { effect_size: '', cles_explanation: '', }, - kde_new: { - median: 0, - sample_count: 0, - kde_x: [], - kde_y: [], - }, - kde_base: { - median: 0, - sample_count: 0, - kde_x: [], - kde_y: [], - }, - silverman_kde: { - bandwidth: '', - base_mode_count: 0, - new_mode_count: 0, - modes: [], - base_locations: [], - new_locations: [], - base_prominence: 0, - new_prominence: 0, - warnings: [], - is_regression: null, - is_improvement: null, - }, - is_fit_good: null, - is_significant: null, is_new_better: null, - performance_intepretation: '', direction_of_change: null, kde_warnings: [], }, @@ -1343,7 +1231,6 @@ const getTestData = () => { delta_percentage: 0.0, is_new_better: null, lower_is_better: true, - is_fit_good: true, more_runs_are_needed: false, direction_of_change: 'regression', is_improvement: null, @@ -1405,7 +1292,6 @@ const getTestData = () => { }, cliffs_delta: 0.0, cliffs_interpretation: 'negligible', - warning_c_delta: null, cles: { cles: 0.2, cles_direction: '', @@ -1415,50 +1301,11 @@ const getTestData = () => { effect_size: '', cles_explanation: '', }, - silverman_kde: { - bandwidth: 'Silverman', - base_mode_count: 1, - new_mode_count: 1, - base_locations: [465888.0, 465422.11, 466353.89], - new_locations: [465888.0, 465422.11, 466353.89], - base_prominence: 0.08, - new_prominence: 0.08, - warnings: [], - modes: [ - { - mode_name: 'Mode 1', - mode_start: '465883.93', - mode_end: '465892.07', - median_shift_summary: null, - ci_low: null, - ci_high: null, - ci_warning: null, - shift: null, - shift_summary: null, - }, - ], - is_regression: null, - is_improvement: null, - }, silverman_warnings: [], - kde_new: { - median: 465888.0, - sample_count: 1, - kde_x: [465888.0, 465422.11, 466353.89], - kde_y: [], - }, - kde_base: { - median: 465888.0, - sample_count: 1, - kde_x: [465888.0, 465422.11, 466353.89], - kde_y: [], - }, kde_warnings: [ 'Less than 2 datapoints or no standard variance for a meaningful fit Kernel Density Estimator (KDE) with an ISJ bandwidth to Base.', 'Less than 2 datapoints or no standard variance for a meaningful fit Kernel Density Estimator (KDE) with an ISJ bandwidth to New.', ], - is_significant: null, - performance_intepretation: '', new_is_better: null, }; @@ -1551,8 +1398,6 @@ const getTestData = () => { }, cliffs_delta: 0.02, cliffs_interpretation: 'negligible', - kde_new: { median: 982.41, sample_count: 2, kde_x: [], kde_y: [] }, - kde_base: { median: 971.38, sample_count: 2, kde_x: [], kde_y: [] }, kde_warnings: [], cles: { cles: 0.6, @@ -1563,23 +1408,7 @@ const getTestData = () => { effect_size: '', cles_explanation: '', }, - silverman_kde: { - bandwidth: '', - base_mode_count: 0, - new_mode_count: 0, - modes: [], - base_locations: [], - new_locations: [], - base_prominence: 0, - new_prominence: 0, - warnings: [], - is_regression: null, - is_improvement: null, - }, - is_fit_good: null, - is_significant: null, is_new_better: null, - performance_intepretation: '', direction_of_change: 'regression', }, { @@ -1677,26 +1506,8 @@ const getTestData = () => { effect_size: '', cles_explanation: '', }, - kde_new: { median: 319.45, sample_count: 2, kde_x: [], kde_y: [] }, - kde_base: { median: 316.35, sample_count: 2, kde_x: [], kde_y: [] }, kde_warnings: [], - silverman_kde: { - bandwidth: '', - base_mode_count: 0, - new_mode_count: 0, - modes: [], - base_locations: [], - new_locations: [], - base_prominence: 0, - new_prominence: 0, - warnings: [], - is_regression: null, - is_improvement: null, - }, - is_fit_good: null, - is_significant: null, is_new_better: null, - performance_intepretation: '', direction_of_change: 'improvement', }, { @@ -1785,26 +1596,8 @@ const getTestData = () => { effect_size: '', cles_explanation: '', }, - kde_new: { median: 982.41, sample_count: 2, kde_x: [], kde_y: [] }, - kde_base: { median: 971.38, sample_count: 2, kde_x: [], kde_y: [] }, kde_warnings: [], - silverman_kde: { - bandwidth: '', - base_mode_count: 0, - new_mode_count: 0, - modes: [], - base_locations: [], - new_locations: [], - base_prominence: 0, - new_prominence: 0, - warnings: [], - is_regression: null, - is_improvement: null, - }, - is_fit_good: null, - is_significant: null, is_new_better: null, - performance_intepretation: '', direction_of_change: null, }, { @@ -1893,26 +1686,8 @@ const getTestData = () => { effect_size: '', cles_explanation: '', }, - kde_new: { median: 984.16, sample_count: 3, kde_x: [], kde_y: [] }, - kde_base: { median: 973.66, sample_count: 3, kde_x: [], kde_y: [] }, kde_warnings: [], - silverman_kde: { - bandwidth: '', - base_mode_count: 0, - new_mode_count: 0, - modes: [], - base_locations: [], - new_locations: [], - base_prominence: 0, - new_prominence: 0, - warnings: [], - is_regression: null, - is_improvement: null, - }, - is_fit_good: null, - is_significant: null, is_new_better: null, - performance_intepretation: '', direction_of_change: null, }, { @@ -2001,26 +1776,8 @@ const getTestData = () => { effect_size: '', cles_explanation: '', }, - kde_new: { median: 984.16, sample_count: 3, kde_x: [], kde_y: [] }, - kde_base: { median: 973.66, sample_count: 3, kde_x: [], kde_y: [] }, kde_warnings: [], - silverman_kde: { - bandwidth: '', - base_mode_count: 0, - new_mode_count: 0, - modes: [], - base_locations: [], - new_locations: [], - base_prominence: 0, - new_prominence: 0, - warnings: [], - is_regression: null, - is_improvement: null, - }, - is_fit_good: null, - is_significant: null, is_new_better: null, - performance_intepretation: '', direction_of_change: null, }, ]; diff --git a/src/components/CompareResults/MannWhitneyCompareMetrics.tsx b/src/components/CompareResults/MannWhitneyCompareMetrics.tsx index 1cead497e..9c0e14850 100644 --- a/src/components/CompareResults/MannWhitneyCompareMetrics.tsx +++ b/src/components/CompareResults/MannWhitneyCompareMetrics.tsx @@ -1,7 +1,6 @@ import { Box } from '@mui/material'; import { MannWhitneyResultsItem } from '../../types/state'; -import { getModeInterpretation } from '../../utils/helpers'; const METRIC_HEADERS = ['Metric', 'Base', 'New', 'Interpretation']; @@ -52,8 +51,6 @@ export const MannWhitneyCompareMetrics = ({ result.shapiro_wilk_test_base?.interpretation ?? 'N/A'; const newShapiroWilkInterpretation = result.shapiro_wilk_test_new?.interpretation ?? 'N/A'; - const baseMode = result?.silverman_kde?.base_mode_count ?? null; - const newMode = result?.silverman_kde?.new_mode_count ?? null; return ( {`${result?.ks_test?.interpretation ?? ''}`} - - Distribution - - - Estimated Modes - {baseMode} - {newMode} - {getModeInterpretation(baseMode, newMode)} - diff --git a/src/components/CompareResults/ResultsTable.tsx b/src/components/CompareResults/ResultsTable.tsx index 3205fb5fe..d2586273b 100644 --- a/src/components/CompareResults/ResultsTable.tsx +++ b/src/components/CompareResults/ResultsTable.tsx @@ -76,7 +76,6 @@ export default function ResultsTable() { searchParams.set('test_version', testVersion); if (testVersion !== MANN_WHITNEY_U) { searchParams.delete('replicates'); - searchParams.delete('enable_silverman_kde'); } setSearchParams(searchParams); }; diff --git a/src/components/CompareResults/hashToCommitLoader.ts b/src/components/CompareResults/hashToCommitLoader.ts index 0f3de1ee5..a6f9a575c 100644 --- a/src/components/CompareResults/hashToCommitLoader.ts +++ b/src/components/CompareResults/hashToCommitLoader.ts @@ -67,9 +67,6 @@ export async function loader({ request }: { request: Request }) { const baseRevsFromHash = commits_from_hashes.baseRevision; const newRevsFromHash = [commits_from_hashes.newRevision]; const replicatesFromUrl = url.searchParams.has('replicates'); - const enableSilvermanKDEFromUrl = url.searchParams.has( - 'enable_silverman_kde', - ); const { baseRev, baseRepo, @@ -79,7 +76,6 @@ export async function loader({ request }: { request: Request }) { frameworkName, replicates, testVersion, - silvermanKDEEnabled, } = checkValues({ baseRev: baseRevsFromHash, baseRepo: baseRepoFromUrl, @@ -88,7 +84,6 @@ export async function loader({ request }: { request: Request }) { framework: frameworkFromUrl, replicates: replicatesFromUrl, testVersion: testVersionFromUrl, - silvermanKDEEnabled: enableSilvermanKDEFromUrl, }); return await getComparisonInformation( baseRev, @@ -98,7 +93,6 @@ export async function loader({ request }: { request: Request }) { frameworkId, frameworkName, replicates, - silvermanKDEEnabled, testVersion, ); } @@ -116,7 +110,6 @@ type HashLoaderData = { view: typeof compareView; generation: number; testVersion: TestVersion; - silvermanKDEEnabled: boolean; }; export type HashLoaderReturnValue = HashLoaderData; diff --git a/src/components/CompareResults/landoToCommitLoader.ts b/src/components/CompareResults/landoToCommitLoader.ts index 7f171613d..3dd29f463 100644 --- a/src/components/CompareResults/landoToCommitLoader.ts +++ b/src/components/CompareResults/landoToCommitLoader.ts @@ -33,9 +33,6 @@ export async function loader({ request }: { request: Request }) { ); } const replicatesFromUrl = url.searchParams.has('replicates'); - const enableSilvermanKDEFromUrl = url.searchParams.has( - 'enable_silverman_kde', - ); const baseRevisionsFromLando = await fetchRevisionFromLandoId( baseLandoIDFromUrl, @@ -57,7 +54,6 @@ export async function loader({ request }: { request: Request }) { frameworkName, replicates, testVersion, - silvermanKDEEnabled, } = checkValues({ baseRev: baseRevisionsFromLando.commit_id, baseRepo: baseRepoFromUrl, @@ -66,7 +62,6 @@ export async function loader({ request }: { request: Request }) { framework: frameworkFromUrl, replicates: replicatesFromUrl, testVersion: testVersionFromUrl, - silvermanKDEEnabled: enableSilvermanKDEFromUrl, }); return await getComparisonInformation( baseRev, @@ -76,7 +71,6 @@ export async function loader({ request }: { request: Request }) { frameworkId, frameworkName, replicates, - silvermanKDEEnabled, testVersion, ); } @@ -94,7 +88,6 @@ type LandoLoaderData = { view: typeof compareView; generation: number; testVersion: TestVersion; - silvermanKDEEnabled: boolean; }; export type LandoLoaderReturnValue = LandoLoaderData; diff --git a/src/components/CompareResults/loader.ts b/src/components/CompareResults/loader.ts index 06916b6e6..353db7eec 100644 --- a/src/components/CompareResults/loader.ts +++ b/src/components/CompareResults/loader.ts @@ -26,7 +26,6 @@ export function checkValues({ framework, replicates, testVersion, - silvermanKDEEnabled, }: { baseRev: string | null; baseRepo: Repository['name'] | null; @@ -35,7 +34,6 @@ export function checkValues({ framework: string | number | null; replicates: boolean; testVersion: TestVersion | null; - silvermanKDEEnabled: boolean; }): { baseRev: string; baseRepo: Repository['name']; @@ -45,7 +43,6 @@ export function checkValues({ frameworkName: Framework['name']; replicates: boolean; testVersion: TestVersion; - silvermanKDEEnabled: boolean; } { if (baseRev === null) { throw new Error('The parameter baseRev is missing.'); @@ -102,7 +99,6 @@ export function checkValues({ frameworkName, replicates, testVersion, - silvermanKDEEnabled, }; } @@ -128,7 +124,6 @@ export function checkValues({ frameworkName, replicates, testVersion, - silvermanKDEEnabled, }; } @@ -142,7 +137,6 @@ async function fetchCompareResultsOnTreeherder({ framework, replicates, testVersion, - silvermanKDEEnabled, }: { baseRev: string; baseRepo: Repository['name']; @@ -151,7 +145,6 @@ async function fetchCompareResultsOnTreeherder({ framework: Framework['id']; replicates: boolean; testVersion?: TestVersion; - silvermanKDEEnabled: boolean; }) { const promises = newRevs.map((newRev, i) => fetchCompareResults({ @@ -162,7 +155,6 @@ async function fetchCompareResultsOnTreeherder({ framework, replicates, testVersion, - silvermanKDEEnabled, }), ); return Promise.all(promises); @@ -234,9 +226,6 @@ export async function loader({ request }: { request: Request }) { const testVersionFromUrl = url.searchParams.get( 'test_version', ) as TestVersion; - const enableSilvermanKDEFromUrl = url.searchParams.has( - 'enable_silverman_kde', - ); const { baseRev, @@ -247,7 +236,6 @@ export async function loader({ request }: { request: Request }) { frameworkName, replicates, testVersion, - silvermanKDEEnabled, } = checkValues({ baseRev: baseRevFromUrl, baseRepo: baseRepoFromUrl, @@ -256,7 +244,6 @@ export async function loader({ request }: { request: Request }) { framework: frameworkFromUrl, replicates: replicatesFromUrl, testVersion: testVersionFromUrl, - silvermanKDEEnabled: enableSilvermanKDEFromUrl, }); return await getComparisonInformation( @@ -267,7 +254,6 @@ export async function loader({ request }: { request: Request }) { frameworkId, frameworkName, replicates, - silvermanKDEEnabled, testVersion, ); } @@ -280,7 +266,6 @@ export async function getComparisonInformation( frameworkId: Framework['id'], frameworkName: Framework['name'], replicates: boolean, - silvermanKDEEnabled: boolean, testVersion?: TestVersion, ) { const resultsPromise = fetchCompareResultsOnTreeherder({ @@ -291,7 +276,6 @@ export async function getComparisonInformation( framework: frameworkId, replicates, testVersion, - silvermanKDEEnabled, }); // TODO what happens if there's no result? @@ -325,7 +309,6 @@ export async function getComparisonInformation( generation: generationCounter++, replicates, testVersion, - silvermanKDEEnabled, }; } @@ -343,7 +326,6 @@ type DeferredLoaderData = { generation: number; replicates: boolean; testVersion: TestVersion; - silvermanKDEEnabled: boolean; }; // Be explicit with the returned type to control it better than if we were diff --git a/src/components/CompareResults/overTimeLoader.ts b/src/components/CompareResults/overTimeLoader.ts index 206d04d04..b47be0ea5 100644 --- a/src/components/CompareResults/overTimeLoader.ts +++ b/src/components/CompareResults/overTimeLoader.ts @@ -26,7 +26,6 @@ function checkValues({ interval, replicates, testVersion, - silvermanKDEEnabled, }: { baseRepo: Repository['name'] | null; newRevs: string[]; @@ -35,7 +34,6 @@ function checkValues({ interval: string | number | null; replicates: boolean; testVersion?: TestVersion | null; - silvermanKDEEnabled: boolean; }): { baseRepo: Repository['name']; newRevs: string[]; @@ -46,7 +44,6 @@ function checkValues({ intervalText: TimeRange['text']; replicates: boolean; testVersion: TestVersion; - silvermanKDEEnabled: boolean; } { if (baseRepo === null) { throw new Error('The parameter baseRepo is missing.'); @@ -127,7 +124,6 @@ function checkValues({ intervalValue, replicates, testVersion, - silvermanKDEEnabled, }; } @@ -141,7 +137,6 @@ async function fetchCompareOverTimeResultsOnTreeherder({ interval, replicates, testVersion, - silvermanKDEEnabled, }: { baseRepo: Repository['name']; newRevs: string[]; @@ -150,7 +145,6 @@ async function fetchCompareOverTimeResultsOnTreeherder({ interval: TimeRange['value']; replicates: boolean; testVersion: TestVersion; - silvermanKDEEnabled: boolean; }) { const promises = newRevs.map((newRev, i) => fetchCompareOverTimeResults({ @@ -161,7 +155,6 @@ async function fetchCompareOverTimeResultsOnTreeherder({ interval, replicates, testVersion, - silvermanKDEEnabled, }), ); return Promise.all(promises); @@ -193,9 +186,6 @@ export async function loader({ request }: { request: Request }) { const testVersionFromUrl = url.searchParams.get( 'test_version', ) as TestVersion; - const enableSilvermanKDEFromUrl = url.searchParams.has( - 'enable_silverman_kde', - ); const { baseRepo, @@ -207,7 +197,6 @@ export async function loader({ request }: { request: Request }) { intervalText, replicates, testVersion, - silvermanKDEEnabled, } = checkValues({ baseRepo: baseRepoFromUrl, newRevs: newRevsFromUrl, @@ -216,7 +205,6 @@ export async function loader({ request }: { request: Request }) { interval: intervalFromUrl, replicates: replicatesFromUrl, testVersion: testVersionFromUrl, - silvermanKDEEnabled: enableSilvermanKDEFromUrl, }); const resultsTimePromise = fetchCompareOverTimeResultsOnTreeherder({ @@ -227,7 +215,6 @@ export async function loader({ request }: { request: Request }) { interval: intervalValue, replicates, testVersion, - silvermanKDEEnabled, }); const newRevsInfoPromises = newRevs.map((newRev, i) => @@ -253,7 +240,6 @@ export async function loader({ request }: { request: Request }) { generation: generationCounter++, replicates, testVersion, - silvermanKDEEnabled, }; } @@ -271,7 +257,6 @@ type DeferredLoaderData = { generation: number; replicates: boolean; testVersion: TestVersion; - silvermanKDEEnabled: boolean; }; // Be explicit with the returned type to control it better than if we were diff --git a/src/components/CompareResults/subtestsLoader.ts b/src/components/CompareResults/subtestsLoader.ts index 2e381b7ea..fcf3800ef 100644 --- a/src/components/CompareResults/subtestsLoader.ts +++ b/src/components/CompareResults/subtestsLoader.ts @@ -15,7 +15,6 @@ function checkValues({ newParentSignature, replicates, testVersion, - silvermanKDEEnabled, }: { baseRev: string | null; baseRepo: Repository['name'] | null; @@ -26,7 +25,6 @@ function checkValues({ newParentSignature: string | null; replicates: boolean; testVersion?: TestVersion; - silvermanKDEEnabled: boolean; }): { baseRev: string; baseRepo: Repository['name']; @@ -38,7 +36,6 @@ function checkValues({ newParentSignature: string; replicates: boolean; testVersion?: TestVersion; - silvermanKDEEnabled: boolean; } { if (baseRev === null) { throw new Error('The parameter baseRev is missing.'); @@ -119,7 +116,6 @@ function checkValues({ newParentSignature, replicates, testVersion, - silvermanKDEEnabled, }; } @@ -146,9 +142,6 @@ export function loader({ request }: { request: Request }) { const replicatesFromUrl = url.searchParams.has('replicates'); const testVersionFromUrl = (url.searchParams.get('test_version') ?? MANN_WHITNEY_U) as TestVersion; - const enableSilvermanKDEFromUrl = url.searchParams.has( - 'enable_silverman_kde', - ); const { baseRev, @@ -161,7 +154,6 @@ export function loader({ request }: { request: Request }) { baseParentSignature, replicates, testVersion, - silvermanKDEEnabled, } = checkValues({ baseRev: baseRevFromUrl, baseRepo: baseRepoFromUrl, @@ -172,7 +164,6 @@ export function loader({ request }: { request: Request }) { newParentSignature: newParentSignatureFromUrl, replicates: replicatesFromUrl, testVersion: testVersionFromUrl, - silvermanKDEEnabled: enableSilvermanKDEFromUrl, }); const results = fetchSubtestsCompareResults({ @@ -185,7 +176,6 @@ export function loader({ request }: { request: Request }) { newParentSignature, replicates, testVersion, - silvermanKDEEnabled, }); return { @@ -200,7 +190,6 @@ export function loader({ request }: { request: Request }) { newParentSignature, replicates, testVersion, - silvermanKDEEnabled, }; } diff --git a/src/components/CompareResults/subtestsOverTimeLoader.tsx b/src/components/CompareResults/subtestsOverTimeLoader.tsx index 906926b21..e45568305 100644 --- a/src/components/CompareResults/subtestsOverTimeLoader.tsx +++ b/src/components/CompareResults/subtestsOverTimeLoader.tsx @@ -20,7 +20,6 @@ function checkValues({ newParentSignature, replicates, testVersion, - silvermanKDEEnabled, }: { baseRepo: Repository['name'] | null; newRev: string | null; @@ -31,7 +30,6 @@ function checkValues({ newParentSignature: string | null; replicates: boolean; testVersion?: TestVersion | null; - silvermanKDEEnabled: boolean; }): { baseRepo: Repository['name']; newRev: string; @@ -44,7 +42,6 @@ function checkValues({ newParentSignature: string; replicates: boolean; testVersion: TestVersion; - silvermanKDEEnabled: boolean; } { if (baseRepo === null) { throw new Error('The parameter baseRepo is missing.'); @@ -144,7 +141,6 @@ function checkValues({ newParentSignature, replicates, testVersion, - silvermanKDEEnabled, }; } @@ -172,9 +168,6 @@ export function loader({ request }: { request: Request }) { const testVersionFromUrl = url.searchParams.get( 'test_version', ) as TestVersion; - const enableSilvermanKDEFromUrl = url.searchParams.has( - 'enable_silverman_kde', - ); const { baseRepo, newRev, @@ -187,7 +180,6 @@ export function loader({ request }: { request: Request }) { newParentSignature, replicates, testVersion, - silvermanKDEEnabled, } = checkValues({ baseRepo: baseRepoFromUrl, newRev: newRevFromUrl, @@ -198,7 +190,6 @@ export function loader({ request }: { request: Request }) { newParentSignature: newParentSignatureFromUrl, replicates: replicatesFromUrl, testVersion: testVersionFromUrl, - silvermanKDEEnabled: enableSilvermanKDEFromUrl, }); const results = fetchSubtestsCompareOverTimeResults({ @@ -211,7 +202,6 @@ export function loader({ request }: { request: Request }) { newParentSignature, replicates, testVersion, - silvermanKDEEnabled, }); return { @@ -227,7 +217,6 @@ export function loader({ request }: { request: Request }) { newParentSignature, replicates, testVersion, - silvermanKDEEnabled, }; } diff --git a/src/hooks/useSubtestRegressionCount.ts b/src/hooks/useSubtestRegressionCount.ts index 70b2ae572..a6db8008c 100644 --- a/src/hooks/useSubtestRegressionCount.ts +++ b/src/hooks/useSubtestRegressionCount.ts @@ -57,7 +57,6 @@ export function useSubtestRegressionCount({ setIsLoading(true); const searchParams = new URLSearchParams(location.search); - const silvermanKDEEnabled = searchParams.has('enable_silverman_kde'); let interval: TimeRange['value'] | null = null; if (view !== compareView) { const intervalStr = searchParams.get('selectedTimeRange'); @@ -78,7 +77,6 @@ export function useSubtestRegressionCount({ newParentSignature: String(new_signature_id), replicates, testVersion, - silvermanKDEEnabled, }) : memoizedFetchSubtestsCompareOverTimeResults({ baseRepo: base_repository_name, @@ -90,7 +88,6 @@ export function useSubtestRegressionCount({ newParentSignature: String(new_signature_id), replicates, testVersion, - silvermanKDEEnabled, }); const strategy: TestVersionStrategy = getStrategy(testVersion); diff --git a/src/logic/treeherder.ts b/src/logic/treeherder.ts index 45fb9bf45..b6921a9c2 100644 --- a/src/logic/treeherder.ts +++ b/src/logic/treeherder.ts @@ -22,7 +22,6 @@ type FetchProps = { framework: Framework['id']; replicates: boolean; testVersion?: TestVersion; - silvermanKDEEnabled?: boolean; }; type FetchOverTimeProps = { @@ -33,7 +32,6 @@ type FetchOverTimeProps = { interval: TimeRange['value']; replicates: boolean; testVersion?: TestVersion; - silvermanKDEEnabled?: boolean; }; type FetchSubtestsProps = { @@ -46,7 +44,6 @@ type FetchSubtestsProps = { newParentSignature: string; replicates: boolean; testVersion?: TestVersion; - silvermanKDEEnabled?: boolean; }; type FetchSubtestsOverTimeProps = { @@ -59,7 +56,6 @@ type FetchSubtestsOverTimeProps = { newParentSignature: string; replicates: boolean; testVersion?: TestVersion; - silvermanKDEEnabled?: boolean; }; export async function fetchRevisionFromHash( @@ -109,7 +105,6 @@ export async function fetchCompareResults({ framework, replicates, testVersion, - silvermanKDEEnabled, }: FetchProps) { const searchParams = new URLSearchParams({ base_repository: baseRepo, @@ -120,7 +115,6 @@ export async function fetchCompareResults({ no_subtests: 'true', replicates: String(replicates), test_version: testVersion ?? STUDENT_T, - enable_silverman_kde: String(silvermanKDEEnabled), }); const url = `${treeherderBaseURL}/api/perfcompare/results/?${searchParams.toString()}`; const response = await fetchFromTreeherder(url); @@ -137,7 +131,6 @@ export async function fetchCompareOverTimeResults({ interval, replicates, testVersion, - silvermanKDEEnabled, }: FetchOverTimeProps) { const searchParams = new URLSearchParams({ base_repository: baseRepo, @@ -148,7 +141,6 @@ export async function fetchCompareOverTimeResults({ no_subtests: 'true', replicates: String(replicates), test_version: testVersion ?? STUDENT_T, - enable_silverman_kde: String(silvermanKDEEnabled), }); const url = `${treeherderBaseURL}/api/perfcompare/results/?${searchParams.toString()}`; const response = await fetchFromTreeherder(url); @@ -167,7 +159,6 @@ export async function fetchSubtestsCompareResults({ newParentSignature, replicates, testVersion, - silvermanKDEEnabled, }: FetchSubtestsProps) { const searchParams = new URLSearchParams({ base_repository: baseRepo, @@ -179,7 +170,6 @@ export async function fetchSubtestsCompareResults({ new_parent_signature: newParentSignature, replicates: String(replicates), test_version: testVersion ?? STUDENT_T, - enable_silverman_kde: String(silvermanKDEEnabled), }); const url = `${treeherderBaseURL}/api/perfcompare/results/?${searchParams.toString()}`; @@ -199,7 +189,6 @@ export async function fetchSubtestsCompareOverTimeResults({ newParentSignature, replicates, testVersion, - silvermanKDEEnabled, }: FetchSubtestsOverTimeProps) { const searchParams = new URLSearchParams({ base_repository: baseRepo, @@ -211,7 +200,6 @@ export async function fetchSubtestsCompareOverTimeResults({ new_parent_signature: newParentSignature, replicates: String(replicates), test_version: testVersion ?? STUDENT_T, - enable_silverman_kde: String(silvermanKDEEnabled), }); const url = `${treeherderBaseURL}/api/perfcompare/results/?${searchParams.toString()}`; diff --git a/src/types/state.ts b/src/types/state.ts index 2091e09ef..4ece99fb9 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -137,13 +137,6 @@ export type StatisticsTestItem = { /* KDE plot values for base or new runs. */ -export type KDEItem = { - median: number; - sample_count: number; - kde_x: number[]; - kde_y: number[]; -}; - /* Common Language Effect Size (CLES) results with Mann-Whitney U test. Interpretation of statistical effect and significance with what level of confidence interval. */ @@ -157,35 +150,6 @@ export type CLESItem = { cles_explanation?: string; } | null; -export type ModeItem = { - mode_name: string; - mode_start?: string; - mode_end?: string; - ci_low?: number | null; - ci_high?: number | null; - ci_warning?: string | null; - shift?: number | null; - shift_summary?: string | null; - median_shift_summary?: string | null; -}; - -/* - Results from Silverman KDE test for multimodal data. -*/ -export type SilvermanKDEItem = { - bandwidth: string; - base_mode_count: number; - new_mode_count: number; - base_locations: number[]; - new_locations: number[]; - base_prominence: number; - new_prominence: number; - warnings: string[]; - modes: ModeItem[]; - is_regression: boolean | null; - is_improvement: boolean | null; -}; - /* Results from the Mann-Whitney U test comparing two sets of runs. */ @@ -221,15 +185,9 @@ export type MannWhitneyResultsItem = { cliffs_delta: number; cliffs_interpretation: string; cles?: CLESItem; // CLES: Common Language Effect Size, statistical effect interpretation from Mann-Whitney U - kde_new: KDEItem; // KDE plots and summary plot with ISJ bandwidth for new runs - kde_base: KDEItem; // KDE plots and summary plot with ISJ bandwidth for base runs kde_warnings: string[]; silverman_warnings?: string[] | null; // silverman warnings about multimodal data - silverman_kde: SilvermanKDEItem; // Silverman KDE multimodal warnings and confidence interval - is_fit_good: boolean | null; // short form interpretation of KS test goodness of fit - is_significant: boolean | null; // is the result statistically significant is_new_better: boolean | null; // is the new revision better than the base revision - performance_intepretation: string; // short text interpretation of the performance change direction_of_change: 'no change' | 'improvement' | 'regression' | null; new_is_better: boolean | null; lower_is_better: boolean | null; @@ -241,7 +199,6 @@ export type MannWhitneyResultsItem = { is_regression: boolean | null; is_meaningful: boolean | null; more_runs_are_needed: boolean | null; - warning_c_delta?: string | null; /* Each test has a signature and each signature may or may not have a parent_signature. If a signature has a parent_signature then we are looking at a subtest. For regular tests this field will be null. diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index 682dc8876..31de1db90 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -124,31 +124,6 @@ const swapArrayElements = ( return array; }; -// Mode interpretation base on mode counts -const getModeInterpretation = ( - baseModeCount: number | null, - newModeCount: number | null, -) => { - const interpretModeCount = (modeCount: number | null) => { - if (!modeCount) return 'N/A'; - if (modeCount === 1) return 'unimodal'; - if (modeCount > 1) return 'multimodal'; - return 'N/A'; - }; - if ( - (!baseModeCount && !newModeCount) || - (baseModeCount === 0 && newModeCount === 0) || - (!baseModeCount && newModeCount === 0) || - (baseModeCount === 0 && !newModeCount) - ) - return 'No modes or data for Base and New, possible oversmoothing, KDE evaluation failed'; - if (baseModeCount && newModeCount && baseModeCount === newModeCount) - return `Base and New revisions are ${interpretModeCount(baseModeCount)}`; - else { - return `Base is ${interpretModeCount(baseModeCount)} and New is ${interpretModeCount(newModeCount)}`; - } -}; - const capitalize = (str: string) => { if (str === '') return ''; return str.charAt(0).toUpperCase() + str.slice(1); @@ -162,6 +137,5 @@ export { swapArrayElements, truncateHash, getDocsURL, - getModeInterpretation, capitalize, };