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 all 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
+1 −1 requirements.txt
102 changes: 59 additions & 43 deletions extension/src/cli/dvc/contract.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Plot } from '../../plots/webview/contract'

export const MIN_CLI_VERSION = '2.52.0'
export const LATEST_TESTED_CLI_VERSION = '2.54.0'
export const MIN_CLI_VERSION = '2.55.0'
export const LATEST_TESTED_CLI_VERSION = '2.55.0'
export const MAX_CLI_VERSION = '3'

type ErrorContents = { type: string; msg: string }
Expand All @@ -26,76 +26,92 @@ export type DataStatusOutput = {
type SingleValue = string | number | boolean | null
export type Value = SingleValue | SingleValue[]

export interface ValueTreeOrError {
data?: ValueTree
error?: ErrorContents
}

type RelPathObject<T> = {
[relPath: string]: T
}

export type ValueTreeRoot = RelPathObject<ValueTreeOrError>
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] I think that having this in the recursive type was actually a bug


export interface ValueTreeNode {
export type ValueTree = {
[key: string]: Value | ValueTree
}

export type ValueTree = ValueTreeRoot | ValueTreeNode

export const isValueTree = (
value: Value | ValueTree
): value is NonNullable<ValueTree> =>
!!(value && !Array.isArray(value) && typeof value === 'object')

export enum ExperimentStatus {
FAILED = 'Failed',
QUEUED = 'Queued',
RUNNING = 'Running',
SUCCESS = 'Success'
FAILED = 'failed',
QUEUED = 'queued',
RUNNING = 'running',
SUCCESS = 'success'
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] These changed on the DVC side.

}

export const EXPERIMENT_WORKSPACE_ID = 'workspace'

export interface BaseExperimentFields {
name?: string
timestamp?: string | null
status?: ExperimentStatus
executor?: string | null
checkpoint_tip?: string
checkpoint_parent?: string
}
export const EXPERIMENT_WORKSPACE_ID = 'workspace' as const

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>
error?: ErrorContents
export type FileDataOrError = { data: ValueTree } | DvcError
export type MetricsOrParams = RelPathObject<FileDataOrError>

export const fileHasError = (file: FileDataOrError): file is DvcError =>
!!(file as DvcError).error

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

export interface ExperimentFieldsOrError {
data?: ExperimentFields
error?: ErrorContents
export enum Executor {
DVC_TASK = 'dvc-task',
WORKSPACE = 'workspace'
}

export interface ExperimentsCommitOutput {
[sha: string]: ExperimentFieldsOrError
baseline: ExperimentFieldsOrError
export type ExecutorState = {
state: ExperimentStatus
name: Executor | null
local: {
root: string | null
log: string | null
pid: number | null
task_id?: string
returncode: null | number
} | null
} | null

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

type ExpWithData = {
rev: string
name?: string
data: ExpData
}

export interface ExperimentsOutput {
[name: string]: ExperimentsCommitOutput
[EXPERIMENT_WORKSPACE_ID]: {
baseline: ExperimentFieldsOrError
}
export type ExpState = ExpWithData | ExpWithError

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

export const experimentHasError = (
expState: ExpState
): expState is ExpWithError => !!(expState as { error?: unknown }).error

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

export interface PlotsData {
[path: string]: Plot[]
}
Expand Down
17 changes: 8 additions & 9 deletions extension/src/cli/dvc/reader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,23 +94,22 @@ describe('CliReader', () => {
})

const cliOutput = await dvcReader.expShow(cwd)
expect(cliOutput).toStrictEqual({
[EXPERIMENT_WORKSPACE_ID]: {
baseline: { error: { msg: unexpectedStderr, type: 'Caught error' } }
expect(cliOutput).toStrictEqual([
{
error: { msg: unexpectedStderr, type: 'Caught error' },
rev: EXPERIMENT_WORKSPACE_ID
}
})
])
})

it('should return the default output if the cli returns an empty object (no commits)', async () => {
it('should return the default output if the cli returns an empty array (no commits)', async () => {
const cwd = __dirname
mockedCreateProcess.mockReturnValueOnce(
getMockedProcess(JSON.stringify({}))
getMockedProcess(JSON.stringify([]))
)

const cliOutput = await dvcReader.expShow(cwd)
expect(cliOutput).toStrictEqual({
[EXPERIMENT_WORKSPACE_ID]: { baseline: {} }
})
expect(cliOutput).toStrictEqual([{ rev: EXPERIMENT_WORKSPACE_ID }])
})
})

Expand Down
22 changes: 8 additions & 14 deletions extension/src/cli/dvc/reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import {
import {
DataStatusOutput,
DvcError,
ExperimentsOutput,
EXPERIMENT_WORKSPACE_ID,
PlotsOutput,
PlotsOutputOrError,
RawPlotsOutput
RawPlotsOutput,
ExpShowOutput
} from './contract'
import { getOptions } from './options'
import { typeCheckCommands } from '..'
Expand All @@ -24,12 +24,8 @@ import { Logger } from '../../common/logger'
import { parseNonStandardJson } from '../../util/json'
import { definedAndNonEmpty } from '../../util/array'

const defaultExperimentsOutput: ExperimentsOutput = {
[EXPERIMENT_WORKSPACE_ID]: { baseline: {} }
}

export const isDvcError = <
T extends ExperimentsOutput | DataStatusOutput | RawPlotsOutput
T extends ExpShowOutput | DataStatusOutput | RawPlotsOutput
>(
dataOrError: T | DvcError
): dataOrError is DvcError =>
Expand Down Expand Up @@ -68,19 +64,17 @@ export class DvcReader extends DvcCli {
public async expShow(
cwd: string,
...flags: (ExperimentFlag | string)[]
): Promise<ExperimentsOutput> {
const output = await this.readProcess<ExperimentsOutput>(
): Promise<ExpShowOutput> {
const output = await this.readProcess<ExpShowOutput>(
cwd,
JSON.stringify(defaultExperimentsOutput),
JSON.stringify([]),
Command.EXPERIMENT,
SubCommand.SHOW,
...flags,
Flag.JSON
)
if (isDvcError(output) || isEmpty(output)) {
return {
[EXPERIMENT_WORKSPACE_ID]: { baseline: output as DvcError | {} }
}
return [{ rev: EXPERIMENT_WORKSPACE_ID, ...(output as DvcError) }]
}
return output
}
Expand Down Expand Up @@ -150,7 +144,7 @@ export class DvcReader extends DvcCli {
}

private async readProcess<
T extends DataStatusOutput | ExperimentsOutput | RawPlotsOutput
T extends DataStatusOutput | 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
15 changes: 13 additions & 2 deletions extension/src/data/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ 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>
public readonly onDidChangeDvcYaml: Event<void>

protected readonly dvcRoot: string
protected readonly processManager: ProcessManager
Expand All @@ -25,6 +26,10 @@ export abstract class BaseData<
new EventEmitter()
)

private readonly dvcYamlChanged: EventEmitter<void> = this.dispose.track(
new EventEmitter<void>()
)

constructor(
dvcRoot: string,
internalCommands: InternalCommands,
Expand All @@ -42,6 +47,8 @@ export abstract class BaseData<
this.onDidUpdate = this.updated.event
this.staticFiles = staticFiles

this.onDidChangeDvcYaml = this.dvcYamlChanged.event

this.watchFiles()

this.waitForInitialData()
Expand Down Expand Up @@ -80,6 +87,10 @@ export abstract class BaseData<
) {
void this.managedUpdate(path)
}

if (path.endsWith('dvc.yaml')) {
this.dvcYamlChanged.fire()
}
}
)
}
Expand Down
Loading