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

ref: document dvc queue #3715

Merged
merged 33 commits into from
Aug 3, 2022
Merged

ref: document dvc queue #3715

merged 33 commits into from
Aug 3, 2022

Conversation

pmrowla
Copy link
Contributor

@pmrowla pmrowla commented Jul 1, 2022

Initial command reference changes for dvc queue and dvc exp
Related to #3658

TODO:

  • dvc exp run changes/deprecations
  • dvc queue examples

@pmrowla pmrowla self-assigned this Jul 1, 2022
@shcheklein shcheklein temporarily deployed to dvc-org-cmdref-dvc-queu-iqfvpe July 1, 2022 07:04 Inactive
@dberenbaum
Copy link
Contributor

@pmrowla Is this ready for review?

@pmrowla
Copy link
Contributor Author

pmrowla commented Jul 2, 2022

@pmrowla Is this ready for review?

Everything that's pushed can be reviewed, but it still needs examples and the exp run changes

@shcheklein shcheklein temporarily deployed to dvc-org-cmdref-dvc-queu-iqfvpe July 5, 2022 06:11 Inactive
@pmrowla pmrowla force-pushed the cmdref-dvc-queue branch from caadb5e to 75e21a3 Compare July 5, 2022 06:14
@shcheklein shcheklein temporarily deployed to dvc-org-cmdref-dvc-queu-iqfvpe July 5, 2022 06:14 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-cmdref-dvc-queu-iqfvpe July 5, 2022 07:31 Inactive
@pmrowla pmrowla changed the title [WIP] ref: document dvc queue ref: document dvc queue Jul 5, 2022
Comment on lines 98 to 104
> ⚠️ `dvc exp run --run-all [--jobs]` is now a shortcut for
> `dvc queue start [--jobs]` followed by `dvc queue logs -f`. The `--run-all`
> and `--jobs` options will likely be deprecated and removed in a future DVC
> release. It is recommended to migrate your workflows to use
> `dvc queue start` and `dvc queue logs`. Refer to the `dvc queue`
> documentation for more details.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also include this kind of warning in core DVC (i.e. actually logging DeprecationWarning)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually yes, but there haven't been any formal decisions about what and what not to deprecate w/the queueing changes. I think the current plan is to not rush formally deprecating the existing experiments UI since the dvc queue workflow will probably change once we get some user feedback

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine to say that this will be deprecated (we are already doing it here). There shouldn't be a hard requirement to include deprecation of experiment features since they are "experimental" in 2.X, but it would be nice.

@pmrowla
Copy link
Contributor Author

pmrowla commented Jul 5, 2022

@dberenbaum @jorgeorpinel this is ready for review.

I've added examples for queue logs and queue status, not sure how detailed we want to get w/the other commands given that we didn't have many examples for queueing at all in the first place.

@shcheklein shcheklein temporarily deployed to dvc-org-cmdref-dvc-queu-iqfvpe August 2, 2022 12:35 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-cmdref-dvc-queu-iqfvpe August 3, 2022 05:27 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-cmdref-dvc-queu-iqfvpe August 3, 2022 05:29 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-cmdref-dvc-queu-iqfvpe August 3, 2022 05:39 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-cmdref-dvc-queu-iqfvpe August 3, 2022 05:40 Inactive
@pmrowla
Copy link
Contributor Author

pmrowla commented Aug 3, 2022

@dberenbaum I can go ahead and continue making all of the suggested copy-edit type changes, but I was under the impression that this PR was going to be merged as-is and then further changes would be taken over by the docs team (and that these review issues were notes @jorgeorpinel was making for future reference)

see: #3715 (comment)

@dberenbaum
Copy link
Contributor

Okay, nothing blocking, so I'll merge as is. I hadn't seen any responses to some of the comments so wasn't sure what was happening.

@shcheklein
Copy link
Member

Product question - do we actually plan to deprecate anything related to "queued experiments" in the dvc exp run and other dvc exp commands? To me it still feels a bit weird (and too low level) that we have to expose dvc queue to also manage experiments. Ideally one interface should be more or less enough (and dvc queue is needed to do more granular, sophisticated tasks).

@dberenbaum
Copy link
Contributor

do we actually plan to deprecate anything related to "queued experiments" in the dvc exp run and other dvc exp commands?

The deprecations in here are for --run-all and -j. I wavered on this but I still leaned towards deprecating those.

My rationale:

  • --run-all is not the most helpful way to start the queue because it runs in the foreground. I think by default we should suggest to start the queue in the background. We could hint to use dvc queue start when an experiment is queued.
  • --run-all and -j are awkward in dvc exp run because they are detached from the typical behavior of dvc exp run and are mutually exclusive from the other options.
  • Although the queue commands should generally be low level, I think dvc queue start is more straightforward than dvc exp run --run-all and provides a general intro to the queue commands in case they are needed later.

@shcheklein
Copy link
Member

@dberenbaum makes total sense. We can still probably expose queue to the experiments level in a different way? e.g. dvc exp run-all ? etc

jorgeorpinel added a commit that referenced this pull request Aug 25, 2022
@jorgeorpinel jorgeorpinel added C: guide Content of /doc/user-guide C: ref Content of /doc/*-reference labels Aug 25, 2022
jorgeorpinel added a commit that referenced this pull request Aug 25, 2022
jorgeorpinel added a commit that referenced this pull request Aug 25, 2022
jorgeorpinel added a commit that referenced this pull request Aug 25, 2022
dberenbaum pushed a commit that referenced this pull request Sep 8, 2022
* guide: update exp queue running examples
per #3715 (review)

* ref: move deprecation note from `exp run --jobs` to
`--run-all` per #3715 (review)

* ref: more context in queue index
per #3715 (review)

* queue: more descriptive start and parallel info
rel. #3715 (review)

* ref: updates to queue kill (et al)

* ref: edits to queue stop

* ref: edit queue remove

* ref: edit queue status

* ref: edit queue logs et al

* guide: keep exp/queue commands
per #3894 (comment)
and #3894 (review)

* guide: roll back typo
per #3894 (review)

* ref: update queue remove intro
per #3894 (comment)

* ref: roll back queue usage
rel #3894 (review)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: docs Area: user documentation (gatsby-theme-iterative) C: guide Content of /doc/user-guide C: ref Content of /doc/*-reference ⌛ status: wait-core-merge Waiting for related product PR merge/release
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants