DVCLive: Deprecate live section of dvc.yaml. - #3411
Conversation
In favor of reusing `metrics` and `plots`. Closes treeverse/dvclive#229
daavoo
added
the
⌛ status: wait-core-merge
daavoo
removed
the
⌛ status: wait-core-merge
| --checkpoints model.pt \ | ||
| --plots-no-cache predictions.json \ | ||
| --live dvclive \ | ||
| --plots-no-cache dvclive/scalars \ |
There was a problem hiding this comment.
Any particular reason to use --plots-no-cache instead of --plots like above?
There was a problem hiding this comment.
I think that we should encourage plots-no-cache in the default example usages (for dvclive/scalars)
There was a problem hiding this comment.
No longer sure about this as default 😅 .
I was thinking it might be better to document the previous default behavior:
metrics:
- dvclive.json:
cache: True
plots:
- dvclive
And later document this flexibility in as an admonition or something.
There was a problem hiding this comment.
What do you mean "as default"? Do you mean this is an implicit recommendation? Well, up to you here @daavoo. I think -no-cache makes more sense for small text files Git can handle but maybe that context is lost in the sample, and regular --plots is def. easier to read.
Maybe the Q is whether
--plotsshould by default not commit outputs to cache, but that's a core Q and I think it was discussed some time ago.
| @@ -97,13 +97,11 @@ $ dvc stage add --name train \ | |||
| --params seed,lr,weight_decay \ | |||
| --checkpoints model.pt \ | |||
| --plots-no-cache predictions.json \ | |||
There was a problem hiding this comment.
Do you know if --plots-no-cache predictions.json is used at all? Seems like that file isn't meant to be a plot and maybe should be --outs-no-cache instead. cc @flippedcoder
dberenbaum
left a comment
dberenbaum
left a comment
There was a problem hiding this comment.
Looks good! I can take another look once deployments are up again.
Co-authored-by: Dave Berenbaum <dave@iterative.ai>
|
Hi. Why was the |
| > This only has an effect when used with `--type=dl`. | ||
| - `--live` - use the given path to set up `metrics` and `plots` as in |
There was a problem hiding this comment.
--type=dl is no longer needed? Is it still a thing? That option desc. may also need an update.
There was a problem hiding this comment.
The thing is that:
This only has an effect when used with
--type=dl
Was not true at all. Passing live can have effect regardless of whether you use --type=dl or not
There was a problem hiding this comment.
OK. So is the --type description accurate?
dl stages are intended for use in deep-learning scenarios, where metrics and plots are tracked with DVCLive.
There was a problem hiding this comment.
It needs to be revisited now that treeverse/dvc#7703 has been merged. It belongs to different P.R. though
Gatsby Cloud Build Reportdvc.org 🎉 Your build was successful! See the Deploy preview here. Build Details🕐 Build time: 1m PerformanceLighthouse report
|
This comment was marked as resolved.
This comment was marked as resolved.
|
Yup! Thanks @julieg18 . Cc @dberenbaum you can see it here: https://dvcorg-deprecatelive.gtsb.io/doc/dvclive |
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Co-authored-by: Restyled.io <commits@restyled.io>
|
Waiting to resolve treeverse/dvc#7534. |
dberenbaum
added
the
⌛ status: wait-core-merge
I see. Probably doesn't have to block this PR tho? |
Yup, you are right, although it will make documenting Edit: opened treeverse/dvc#7703 to move it along. |
dberenbaum
removed
the
⌛ status: wait-core-merge
|
@dberenbaum @jorgeorpinel could you take another look to see what's missing? I have addressed everything except for #3411 (comment) , which I believe requires separate P.R. to update (as |
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
6 participants
In favor of reusing
metricsandplots.Closes treeverse/dvclive#77
Closes treeverse/dvclive#229
Closes treeverse/dvclive#230