iterative / example-repos-dev

Source code and generator scripts for example DVC projects
https://dvc.org/doc
21 stars 13 forks source link

example-dvc-experiments: Added modifications for DVCLive #95

Closed iesahin closed 2 years ago

iesahin commented 2 years ago

Closes #81

iesahin commented 2 years ago

In the light of changes required, do you think it's best to put these into example-dvc-experiments repository? @daavoo Could we put to another branch? I tried to make minimum changes but this one will be the final state of the repository. These changes will make "dvc exp w/o dvclive" useless. It will be like DVCLive is required to run the experiments.

WDYT @dberenbaum @shcheklein

shcheklein commented 2 years ago

I'm personally fine to depend on dvclive. I would though put a comment in code that let's say "dvclive.json" is the same (or could be replaced with):

with open ...
    f.write(...

and dvclive plays a role of a lighthweight helper to write metric files.

dberenbaum commented 2 years ago

These changes will make "dvc exp w/o dvclive" useless.

I'm not sure what you mean and what your concerns are. Is it that dvclive becomes a dependency of the project? Is it that the project structure has to change? Something else?

iesahin commented 2 years ago

I'm not sure what you mean and what your concerns are. Is it that dvclive becomes a dependency of the project? Is it that the project structure has to change? Something else?

Someone that git clone https://.../example-dvc-experiments will be using a dvclive version instead of a basic dvc exp version. This may look like the experiments need dvclive to run. That's not an important concern actually, and I think merging the live features into DVC may be a better strategy in the longer term for both dvc exp and live.

dberenbaum commented 2 years ago

I agree with @shcheklein that it's fine to start with dvclive since this is the "happy path" as long as we can make clear that this is a convenience rather than a necessity. We might want to include dvclive as a dvc dependency so that users don't need to install multiple packages.

iesahin commented 2 years ago

I'm updating the project to have DVCLive from the start, and all related functionality will use DVCLive, not custom metrics/plots. @shcheklein @dberenbaum

iesahin commented 2 years ago

Now we can make a decision between this (using dvclive) and #97 (using dvc exp init). There is also a third option not to change anything from the example projects and use master as is.

WDYT? @shcheklein @dberenbaum @daavoo @jorgeorpinel

daavoo commented 2 years ago

Now we can make a decision between this (using dvclive) and #97 (using dvc exp init). There is also a third option not to change anything from the example projects and use master as is.

WDYT? @shcheklein @dberenbaum @daavoo @jorgeorpinel

dvclive and dvc exp init should not be incompatible so, why not both?

dvc exp init --type dl would enable dvclive

dberenbaum commented 2 years ago

dvc exp init --type dl would enable dvclive

I'm not sure that's the best intro here since it also introduces checkpoints, which create a different workflow. We could use dvclive without --type dl, either using dvc exp init --live somepath to use the live section in dvc.yaml or by having the metrics and/or plots sections of dvc.yaml align with the dvclive outputs. If we do this, I think we need to make clear somewhere how users can do this without dvclive, or at least without a built-in callback.

daavoo commented 2 years ago

dvc exp init --type dl would enable dvclive

I'm not sure that's the best intro here since it also introduces checkpoints, which create a different workflow. We could use dvclive without --type dl, either using dvc exp init --live somepath to use the live section in dvc.yaml or by having the metrics and/or plots sections of dvc.yaml align with the dvclive outputs. If we do this, I think we need to make clear somewhere how users can do this without dvclive, or at least without a built-in callback.

It seems that the --live flag only has an effect if --type=dl:

https://github.com/iterative/dvc/blob/82c5caee27d4b5591d4ab0b07fd1a73064ba8bff/dvc/repo/experiments/init.py#L218

dberenbaum commented 2 years ago

It seems that the --live flag only has an effect if --type=dl:

The intended behavior is a bit unclear, but dvc exp init --live somepath will include the live section. Regardless, this might be confusing. I'd vote to use the normal metrics and/or plots sections but generate them through dvclive calls. That's what I ended up doing for the dvc exp init demo.

iesahin commented 2 years ago

I replace initial dvc stage add command with a similar dvc exp init, without changing the flow of repo generation. However, I'm getting:

ERROR: failed to reproduce 'dvc.yaml': Checkpoint stages are not supported in 'dvc repro'. Checkpoint stages must be reproduced with 'dvc exp run' or 'dvc exp resume'.

Now, we have iterative/dvc#6592 that leads to WARNINGs to appear for users regarding the model file, and I began to use dvc repro for the first experiment to prevent them. It looks now we are at a point to select either dvc exp init with these warnings, or dvc repro without dvc exp init.

I made some simplifications in the text, in iterative/dvc.org#3051 but I still believe, we can postpone the major update until these issues are resolved. @dberenbaum @shcheklein

dberenbaum commented 2 years ago

@iesahin I'm fine if you think it's not worthwhile to include exp init and/or dvclive yet in the get started doc.

With regards to the error you mention, it seems like you have checkpoint: true somewhere in your dvc.yaml, which I thought from the discussion above was out of scope.

iesahin commented 2 years ago

closed in favor of #97