Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
ref:
exp init
improvements #3071ref:
exp init
improvements #3071Changes from all commits
153d526
38099d3
0509a51
97592c7
e70ea6d
a97e611
8b89b8f
2273431
3d91656
69c8082
8f21601
be7a5af
600e83c
6e3d2fa
42aa486
5e09813
56f4a46
fac16bd
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant this (per last check box in this PR desc.) @dberenbaum @skshetry . Currently
dvc exp init -h
printsInitialize DVC in the current directory. Expects directory to be a Git repository...
Do we want to update that text?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's the
dvc init -h
text 😄 .dvc exp init -h
saysInitialize experiments
. However, I think we agreed this new language is better, so I'm happy to put in a PR to update in the core repo.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep 🤦 sry. OK, thanks for the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhere in the description, it might be useful to explain that
dvc exp init
by default expects that input data, parameters, and source code paths exist before running an experiment, and that the command is expected to generate models, metrics, and plots.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's more or a problem for
exp run
though, @dberenbaum (already linked form the--run
option). But should we state thatexp run
(andrepro
for that matter) expect that the stage definition and code are good? Hopefully its evident.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well that's the point of
exp init
, right? Better to have users understand what's needed up front than to have them runexp init
only to fail onexp run
.Doesn't need to be part of this PR. It could also be handled by some of the suggested changes to the core command rather than the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure. If users need to read that on the cmd ref., is
exp init
serving as a self-explanatory command? I think those notes should be in the command's output fist (and added to this doc as a result of that product update).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW a question I have is whether to keep this section or move
command
under Options like we did fortargets
in https://dvc.org/doc/command-reference/repro#options. I personally like the section more but I remember we discussed it (cc @shcheklein) this and using Options was picked, so for consistency I'd move this under Options as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not a model training example? Data ingestion is a nice application to show how this can be useful even for non-training stages, but model training is the primary use case. It's unlikely for someone to run experiments on a data ingestion script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data ingestion example produced a very small dvc.yaml file and then I had that note about using
stage add
to produce the same. Idk, I thought it was interesting. Anyway, I agree the training scenario is more applicable so I replaced all that now.