Create `DVC` experiment on `live.end`. #366

daavoo · 2022-11-22T10:46:26Z

Closes #310
Closes #311

Requires iterative/dvc#8599 and iterative/dvc#8529

src/dvclive/utils.py

src/dvclive/live.py

codecov-commenter · 2022-11-24T13:35:28Z

Codecov Report

Base: 96.63% // Head: 95.23% // Decreases project coverage by -1.40% ⚠️

Coverage data is based on head (82a4440) compared to base (e9ed5a0).
Patch coverage: 97.15% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #366      +/-   ##
==========================================
- Coverage   96.63%   95.23%   -1.41%     
==========================================
  Files          36       37       +1     
  Lines        1811     1950     +139     
  Branches      160      151       -9     
==========================================
+ Hits         1750     1857     +107     
- Misses         35       66      +31     
- Partials       26       27       +1

Impacted Files	Coverage Δ
src/dvclive/studio.py	`91.11% <ø> (ø)`
src/dvclive/report.py	`96.38% <33.33%> (ø)`
src/dvclive/dvc.py	`88.05% <92.10%> (+4.18%)`	⬆️
src/dvclive/live.py	`96.72% <98.79%> (+0.59%)`	⬆️
src/dvclive/env.py	`100.00% <100.00%> (ø)`
src/dvclive/lightning.py	`90.90% <100.00%> (ø)`
tests/test_dvc.py	`100.00% <100.00%> (ø)`
tests/test_main.py	`100.00% <100.00%> (ø)`
tests/test_report.py	`100.00% <100.00%> (ø)`
tests/test_studio.py	`100.00% <100.00%> (ø)`
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

daavoo · 2022-11-30T16:48:48Z

src/dvclive/live.py

+            self._inside_dvc_exp = True
+        else:
+            # `Python Only` execution
+            # TODO: How to handle `dvc repro` execution?


@dberenbaum what do you think we should do if DVCLive is used inside a dvc pipeline that has been executed with dvc repro?

I think we should skip the experiment creation.

Yes, let's skip it. We may still want to call make_dvcyaml, but we can skip that also for now if it simplifies things.

daavoo · 2022-11-30T16:54:38Z

tests/test_dvc.py

+    assert load_yaml(live.dvc_file) == {
+        "metrics": ["metrics.json"],
+        "params": ["params.yaml"],
+        "plots": ["plots"],


Follow up: configure sklearn plots

src/dvclive/dvc.py

dberenbaum · 2022-11-30T17:05:42Z

src/dvclive/dvc.py

+
+
+def make_dvcyaml(live):
+    if not os.path.exists(live.dvc_file):


Will dvc.yaml get deleted each time a new live instance is created? Maybe we need some option to configure whether to overwrite it. In some cases, I may log new stuff and want to overwrite, but in others I might have added some custom config that I want to keep.

Will dvc.yaml get deleted each time a new live instance is created?

It should not be deleted and it should preserve the manual modifications from users.

In some cases, I may log new stuff and want to overwrite, but in others I might have added some custom config that I want to keep.

What would be overwritten? The current version is extremely simple as it doesn't configure sklearn plots (added a follow-up #371)

Sounds good. Not a blocker.

What would be overwritten?

As you say, it will include sklearn plots config, which seems like the most complicated scenario, so let's discuss there.

It should probably just write to the root of the repo, not to its own file by default. It is the dvc way, and this is against the principle of having one file to rule them all.

🤔 But the dvclive way is to create a self-contained folder 😄 . IMHO dvclive should not write to the user's root dvc.yaml file, because it it makes it hard to share only the dvclive output, and it mixes machine-generated config with user-generated config. Also, if there is some conflict between the existing config and what dvclive generates, I would rather have dvclive run successfully and be able to later debug the conflicts between the files.

dberenbaum · 2022-11-30T17:14:54Z

src/dvclive/live.py

+                self._exp_name = random_exp_name(
+                    self._dvc_repo, self._baseline_rev
+                )
+                make_dvcyaml(self)


I'm not as sure that we should skip make_dvcyaml, but we can follow up on this separate from this PR.

Another follow up: pass an experiment name to use.

And another: consider whether to include the dvclive user script/notebook in the tracked files.

And another: consider whether to include the dvclive user script/notebook in the tracked files.

Do you mean to include it in the include_tracked list passed to experiments save or other thing?

Yes, whether to include it in include_tracked. I'm not sure it's needed, so it was more of a discussion point than a request. There's something clean about the default being to only save the live dir.

dberenbaum

Looks great so far! Very cool to see this come to life.

dberenbaum · 2022-11-30T18:13:33Z

Demo from @daavoo:

exp-onboarding.mp4

Enable with `save_dvc_exp=True`. Defaults to `False`. Refactor `__init__` method. Split into private `_init_{component}` methods. Add `_` prefix to private properties. Use env vars from iterative/dvc#8630 to skip Studio `start` and `done` events. Use same env vars to skip creating DVC exp.

daavoo · 2022-12-01T17:31:36Z

src/dvclive/lightning.py

@@ -17,11 +17,16 @@ def __init__(
        dir: Optional[str] = None,  # noqa pylint: disable=redefined-builtin
        resume: bool = False,
        report: Optional[str] = "auto",
+        save_dvc_exp: bool = False,


@dberenbaum named the option like this. wdyt?

I don't know if it makes sense for the dvc.yaml saving part and whether those should be coupled together, but I think it's fine for now.

I though your concerns applied to both the git ref and the dvc.yaml creation

daavoo self-assigned this Nov 22, 2022

daavoo requested a review from dberenbaum November 22, 2022 10:46

daavoo force-pushed the exp-save branch 3 times, most recently from 58ca881 to 802c503 Compare November 22, 2022 10:55

daavoo mentioned this pull request Nov 22, 2022

Improve experiments naming iterative/vscode-dvc#2457

Closed

daavoo force-pushed the exp-save branch from 802c503 to aac6fc5 Compare November 22, 2022 11:06

dberenbaum reviewed Nov 22, 2022

View reviewed changes

src/dvclive/utils.py Outdated Show resolved Hide resolved

dberenbaum reviewed Nov 22, 2022

View reviewed changes

src/dvclive/live.py Outdated Show resolved Hide resolved

daavoo force-pushed the exp-save branch 5 times, most recently from 7714f8b to 965de7d Compare November 24, 2022 13:25

daavoo force-pushed the exp-save branch from 965de7d to aede7f2 Compare November 30, 2022 16:39

daavoo added A: dvc DVC integration feature labels Nov 30, 2022

daavoo force-pushed the exp-save branch from aede7f2 to 75fc549 Compare November 30, 2022 16:40

daavoo changed the title ~~Save results as DVC experiment~~ Create DVC experiment on live.end. Nov 30, 2022

daavoo force-pushed the exp-save branch from 75fc549 to 47fc2e6 Compare November 30, 2022 16:45

daavoo marked this pull request as ready for review November 30, 2022 16:46

daavoo commented Nov 30, 2022

View reviewed changes

daavoo force-pushed the exp-save branch from 47fc2e6 to 6732b84 Compare November 30, 2022 16:53

daavoo commented Nov 30, 2022

View reviewed changes

daavoo mentioned this pull request Nov 30, 2022

dvc: Configure sklearn plots in generated dvc.yaml. #371

Closed

daavoo requested a review from dberenbaum November 30, 2022 16:55

daavoo commented Nov 30, 2022

View reviewed changes

src/dvclive/dvc.py Outdated Show resolved Hide resolved

daavoo force-pushed the exp-save branch from 6732b84 to c8c2155 Compare November 30, 2022 17:03

dberenbaum reviewed Nov 30, 2022

View reviewed changes

dberenbaum approved these changes Nov 30, 2022

View reviewed changes

dberenbaum added the ⌛ status: wait-core-merge label Nov 30, 2022

dberenbaum mentioned this pull request Nov 30, 2022

Experiments onboarding iterative/dvc.org#4149

Closed

7 tasks

daavoo force-pushed the exp-save branch from c8c2155 to 0f8d289 Compare November 30, 2022 18:44

This was referenced Nov 30, 2022

dvc: Allow to pass experiment name #373

Closed

log_artifact #378

Closed

daavoo force-pushed the exp-save branch from 0f8d289 to 236629e Compare December 1, 2022 15:00

dvc: Add utils for saving experiments.

429d9ba

daavoo force-pushed the exp-save branch from a276629 to 7d7cac6 Compare December 1, 2022 15:21

daavoo force-pushed the exp-save branch from 7d7cac6 to 82a4440 Compare December 1, 2022 17:16

daavoo commented Dec 1, 2022

View reviewed changes

daavoo removed the ⌛ status: wait-core-merge label Dec 1, 2022

daavoo merged commit 63d1b20 into main Dec 1, 2022

daavoo deleted the exp-save branch December 1, 2022 18:14

daavoo mentioned this pull request Jul 4, 2023

cleanup dvcyaml #615

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create `DVC` experiment on `live.end`. #366

Create `DVC` experiment on `live.end`. #366

daavoo commented Nov 22, 2022 •

edited

Loading

codecov-commenter commented Nov 24, 2022 •

edited

Loading

daavoo Nov 30, 2022

dberenbaum Nov 30, 2022

daavoo Nov 30, 2022

dberenbaum Nov 30, 2022

daavoo Nov 30, 2022

dberenbaum Nov 30, 2022

skshetry Dec 1, 2022

dberenbaum Dec 1, 2022

dberenbaum Nov 30, 2022

dberenbaum Nov 30, 2022

dberenbaum Nov 30, 2022

daavoo Nov 30, 2022

dberenbaum Nov 30, 2022

dberenbaum left a comment

dberenbaum commented Nov 30, 2022

daavoo Dec 1, 2022

dberenbaum Dec 1, 2022

daavoo Dec 1, 2022 •

edited

Loading



		def make_dvcyaml(live):
		if not os.path.exists(live.dvc_file):

Create DVC experiment on live.end. #366

Create DVC experiment on live.end. #366

Conversation

daavoo commented Nov 22, 2022 • edited Loading

codecov-commenter commented Nov 24, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dberenbaum left a comment

Choose a reason for hiding this comment

dberenbaum commented Nov 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daavoo Dec 1, 2022 • edited Loading

Choose a reason for hiding this comment

Create `DVC` experiment on `live.end`. #366

Create `DVC` experiment on `live.end`. #366

daavoo commented Nov 22, 2022 •

edited

Loading

codecov-commenter commented Nov 24, 2022 •

edited

Loading

daavoo Dec 1, 2022 •

edited

Loading