Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump min DVC version to 2.55.0 (live metrics for experiments running outside the workspace) #3665

Merged
merged 25 commits into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
61bed4c
duplicate transform and set function with new data
mattseddon Apr 11, 2023
e8415a0
duplicate functions out to reader (tests will break)
mattseddon Apr 11, 2023
351e42a
fix all integration tests
mattseddon Apr 12, 2023
2bbe80f
update all remaining test fixtures
mattseddon Apr 13, 2023
6b77df0
fix all test data and deduplicate functions
mattseddon Apr 13, 2023
4be5754
Merge branch 'main' into integrate-exp-show
mattseddon Apr 13, 2023
679c7ce
remove some code climate duplication
mattseddon Apr 13, 2023
336d737
Merge branch 'main' into integrate-exp-show
mattseddon Apr 13, 2023
9c5f59c
wrap all loose test data in generator
mattseddon Apr 14, 2023
b198d11
use breaking change version of dvc
mattseddon Apr 14, 2023
8340d74
fix experiment status in dvc contract
mattseddon Apr 14, 2023
54eedd9
self review
mattseddon Apr 17, 2023
ead10f9
Merge branch 'main' into integrate-exp-show
mattseddon Apr 17, 2023
269840a
Remove checkpoints model and file system watcher (#3684)
mattseddon Apr 17, 2023
ce57701
Merge branch 'main' into integrate-exp-show
mattseddon Apr 17, 2023
a9f5ee5
update demo project
mattseddon Apr 19, 2023
932951b
Merge branch 'main' into integrate-exp-show
mattseddon Apr 19, 2023
5804573
Extend timeout of run experiment test (e2e) (#3713)
mattseddon Apr 19, 2023
ef25c85
Prevent plotting of running experiments (#3712)
mattseddon Apr 19, 2023
8fee434
Merge branch 'main' into integrate-exp-show
mattseddon Apr 19, 2023
6f3fd93
Trigger plot updates whenever commit data changes (#3715)
mattseddon Apr 19, 2023
d275ab0
Merge branch 'main' into integrate-exp-show
mattseddon Apr 19, 2023
cb72502
update demo project and min required version of DVC
mattseddon Apr 20, 2023
ccdfc04
Fix experiment id for commits (shown in plots) (#3724)
mattseddon Apr 20, 2023
b2c2835
Merge branch 'main' into integrate-exp-show
mattseddon Apr 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion demo
Submodule demo updated 1 files
+2 −2 requirements.txt
78 changes: 73 additions & 5 deletions extension/src/cli/dvc/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export enum ExperimentStatus {
SUCCESS = 'Success'
}

export const EXPERIMENT_WORKSPACE_ID = 'workspace'
export const EXPERIMENT_WORKSPACE_ID = 'workspace' as const

export interface BaseExperimentFields {
name?: string
Expand All @@ -69,13 +69,14 @@ export interface BaseExperimentFields {
type Dep = { hash: null | string; size: null | number; nfiles: null | number }
type Out = Dep & { use_cache: boolean; is_data_source: boolean }

type Outs = RelPathObject<Out>
export type Deps = RelPathObject<Dep>

export interface ExperimentFields extends BaseExperimentFields {
params?: ValueTreeRoot
metrics?: ValueTreeRoot
deps?: Deps
outs?: RelPathObject<Out>
params?: ValueTreeRoot | null
metrics?: ValueTreeRoot | null
deps?: Deps | null
outs?: RelPathObject<Out> | null
error?: ErrorContents
}

Expand All @@ -96,6 +97,73 @@ export interface ExperimentsOutput {
}
}

type ValueTreeOrError_ = { data: ValueTree } | DvcError
type MetricsOrParams = RelPathObject<ValueTreeOrError_>

export type ExpStateData = {
rev: string
timestamp: string | null
params: MetricsOrParams | null
metrics: MetricsOrParams | null
deps: Deps | null
outs: Outs | null
meta: { has_checkpoints: boolean }
}

export const ExperimentExecutor = {
DVC_TASK: 'dvc-task',
[EXPERIMENT_WORKSPACE_ID]: EXPERIMENT_WORKSPACE_ID
} as const

export type Executor =
| {
state: ExperimentStatus.QUEUED
name: typeof ExperimentExecutor.DVC_TASK
local: {
root: null
log: null
pid: null
returncode: null
task_id: string
}
}
| {
state:
| ExperimentStatus.RUNNING
| ExperimentStatus.FAILED
| ExperimentStatus.SUCCESS
name: (typeof ExperimentExecutor)[keyof typeof ExperimentExecutor] | null
local: {
root: string
log: string
pid: number
task_id?: string
returncode: null | number
} | null
}
| null

export type ExpWithError = {
rev: string
name?: string
} & DvcError

export type ExpState =
| {
rev: string
name?: string
data: ExpStateData
}
| ExpWithError

export type ExpRange = {
revs: ExpState[]
name?: string
executor: Executor
}

export type ExpShowOutput = (ExpState & { experiments?: ExpRange[] | null })[]

export interface PlotsData {
[path: string]: Plot[]
}
Expand Down
2 changes: 1 addition & 1 deletion extension/src/cli/dvc/reader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('CliReader', () => {
getMockedProcess(JSON.stringify(expShowFixture))
)

const cliOutput = await dvcReader.expShow(cwd)
const cliOutput = await dvcReader.expShow_(cwd)
expect(cliOutput).toStrictEqual(expShowFixture)
expect(mockedCreateProcess).toHaveBeenCalledWith({
args: ['exp', 'show', JSON_FLAG],
Expand Down
34 changes: 31 additions & 3 deletions extension/src/cli/dvc/reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {
EXPERIMENT_WORKSPACE_ID,
PlotsOutput,
PlotsOutputOrError,
RawPlotsOutput
RawPlotsOutput,
ExpShowOutput
} from './contract'
import { getOptions } from './options'
import { typeCheckCommands } from '..'
Expand All @@ -29,7 +30,11 @@ const defaultExperimentsOutput: ExperimentsOutput = {
}

export const isDvcError = <
T extends ExperimentsOutput | DataStatusOutput | RawPlotsOutput
T extends
| ExperimentsOutput
| ExpShowOutput
| DataStatusOutput
| RawPlotsOutput
>(
dataOrError: T | DvcError
): dataOrError is DvcError =>
Expand All @@ -38,6 +43,7 @@ export const isDvcError = <
export const autoRegisteredCommands = {
DATA_STATUS: 'dataStatus',
EXP_SHOW: 'expShow',
EXP_SHOW_: 'expShow_',
GLOBAL_VERSION: 'globalVersion',
PLOTS_DIFF: 'plotsDiff',
ROOT: 'root',
Expand Down Expand Up @@ -65,6 +71,24 @@ export class DvcReader extends DvcCli {
)
}

public async expShow_(
cwd: string,
...flags: (ExperimentFlag | string)[]
): Promise<ExpShowOutput> {
const output = await this.readProcess<ExpShowOutput>(
cwd,
JSON.stringify([]),
Command.EXPERIMENT,
SubCommand.SHOW,
...flags,
Flag.JSON
)
if (isDvcError(output) || isEmpty(output)) {
return [{ rev: EXPERIMENT_WORKSPACE_ID, ...(output as DvcError) }]
}
return output
}

public async expShow(
cwd: string,
...flags: (ExperimentFlag | string)[]
Expand Down Expand Up @@ -150,7 +174,11 @@ export class DvcReader extends DvcCli {
}

private async readProcess<
T extends DataStatusOutput | ExperimentsOutput | RawPlotsOutput
T extends
| DataStatusOutput
| ExperimentsOutput
| ExpShowOutput
| RawPlotsOutput
>(cwd: string, defaultValue: string, ...args: Args): Promise<T | DvcError> {
try {
const output =
Expand Down
6 changes: 3 additions & 3 deletions extension/src/cli/dvc/viewer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ export class DvcViewer extends Disposable implements ICli {
return this.createProcess(name, cwd, args)
}

public queueLogs(cwd: string, expName: string) {
public queueLogs(cwd: string, id: string) {
return this.run(
`${expName} logs`,
`${id} logs`,
cwd,
Command.QUEUE,
QueueSubCommand.LOGS,
expName,
id,
Flag.FOLLOW
)
}
Expand Down
4 changes: 2 additions & 2 deletions extension/src/data/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import { getRelativePattern } from '../fileSystem/relativePattern'
import { createFileSystemWatcher } from '../fileSystem/watcher'
import { ProcessManager } from '../process/manager'
import { InternalCommands } from '../commands/internal'
import { ExperimentsOutput, PlotsOutputOrError } from '../cli/dvc/contract'
import { ExpShowOutput, PlotsOutputOrError } from '../cli/dvc/contract'
import { uniqueValues } from '../util/array'
import { DeferredDisposable } from '../class/deferred'
import { isSameOrChild } from '../fileSystem'

export abstract class BaseData<
T extends { data: PlotsOutputOrError; revs: string[] } | ExperimentsOutput
T extends { data: PlotsOutputOrError; revs: string[] } | ExpShowOutput
> extends DeferredDisposable {
public readonly onDidUpdate: Event<T>

Expand Down
96 changes: 24 additions & 72 deletions extension/src/experiments/columns/collect/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
/* eslint-disable sort-keys-fix/sort-keys-fix */
import { join } from 'path'
import { collectChanges, collectColumns } from '.'
import {
collectChanges,
collectChanges_,
collectColumns,
collectColumns_,
collectRelativeMetricsFiles
} from '.'
import { timestampColumn } from '../constants'
import { buildDepPath, buildMetricOrParamPath } from '../paths'
import { buildMetricOrParamPath } from '../paths'
import { Column, ColumnType } from '../../webview/contract'
import outputFixture from '../../../test/fixtures/expShow/base/output'
import columnsFixture from '../../../test/fixtures/expShow/base/columns'
Expand All @@ -12,7 +17,8 @@ import {
ExperimentsOutput,
ExperimentStatus,
EXPERIMENT_WORKSPACE_ID,
ValueTree
ValueTree,
ExpStateData
} from '../../../cli/dvc/contract'
import { getConfigValue } from '../../../vscode/config'

Expand All @@ -23,7 +29,7 @@ mockedGetConfigValue.mockImplementation(() => 5)

describe('collectColumns', () => {
it('should return a value equal to the columns fixture when given the output fixture', () => {
const columns = collectColumns(outputFixture)
const columns = collectColumns_(outputFixture)
expect(columns).toStrictEqual(columnsFixture)
})

Expand Down Expand Up @@ -429,70 +435,6 @@ describe('collectColumns', () => {

expect(onlyHasPrimitiveChild).toBeUndefined()
})

it('should collect all params and metrics from the test fixture', () => {
expect(collectColumns(outputFixture).map(({ path }) => path)).toStrictEqual(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[F] This is a duplicate of should return a value equal to the columns fixture when given the output fixture

[
timestampColumn.path,
buildMetricOrParamPath(ColumnType.METRICS, 'summary.json'),
buildMetricOrParamPath(ColumnType.METRICS, 'summary.json', 'loss'),
buildMetricOrParamPath(ColumnType.METRICS, 'summary.json', 'accuracy'),
buildMetricOrParamPath(ColumnType.METRICS, 'summary.json', 'val_loss'),
buildMetricOrParamPath(
ColumnType.METRICS,
'summary.json',
'val_accuracy'
),
buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml'),
buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml', 'code_names'),
buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml', 'epochs'),
buildMetricOrParamPath(
ColumnType.PARAMS,
'params.yaml',
'learning_rate'
),
buildMetricOrParamPath(
ColumnType.PARAMS,
'params.yaml',
'dvc_logs_dir'
),
buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml', 'log_file'),
buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml', 'dropout'),
buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml', 'process'),
buildMetricOrParamPath(
ColumnType.PARAMS,
'params.yaml',
'process',
'threshold'
),
buildMetricOrParamPath(
ColumnType.PARAMS,
'params.yaml',
'process',
'test_arg'
),
buildMetricOrParamPath(
ColumnType.PARAMS,
join('nested', 'params.yaml')
),
buildMetricOrParamPath(
ColumnType.PARAMS,
join('nested', 'params.yaml'),
'test'
),
buildDepPath('data'),
buildDepPath('data', 'data.xml'),
buildDepPath('src'),
buildDepPath('src', 'prepare.py'),
buildDepPath('data', 'prepared'),
buildDepPath('src', 'featurization.py'),
buildDepPath('data', 'features'),
buildDepPath('src', 'train.py'),
buildDepPath('model.pkl'),
buildDepPath('src', 'evaluate.py')
]
)
})
})

describe('collectChanges', () => {
Expand All @@ -514,16 +456,18 @@ describe('collectChanges', () => {
}

it('should mark new dep files as changes', () => {
const changes = collectChanges(uncommittedDepsFixture)
const changes = collectChanges_(uncommittedDepsFixture)
expect(changes).toStrictEqual(
Object.keys(uncommittedDepsFixture.workspace.baseline.data?.deps || {})
Object.keys(
(uncommittedDepsFixture[0] as { data: ExpStateData }).data.deps || {}
)
.map(dep => `deps:${dep}`)
.sort()
)
})

it('should return the expected data from the output fixture', () => {
const changes = collectChanges(outputFixture)
const changes = collectChanges_(outputFixture)
expect(changes).toStrictEqual(workspaceChangesFixture)
})

Expand Down Expand Up @@ -821,3 +765,11 @@ describe('collectChanges', () => {
])
})
})

describe('collectRelativeMetricsFiles', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[F] This was moved from plots/data/collect to encapsulate the data type inside of Experiments.

it('should return the expected metrics files from the test fixture', () => {
expect(collectRelativeMetricsFiles(outputFixture)).toStrictEqual([
'summary.json'
])
})
})
Loading