queue: edits to new docs
#3894
queue: edits to new docs
#3894
Conversation
jorgeorpinel
added
A: docs
jorgeorpinel
changed the title
queue: edits to new docs
jorgeorpinel
force-pushed
the
queue/edits
branch
from
96df85f to
3f7fd5e
Compare
| When you've created experiments to be run in the queue with | ||
| `dvc exp run --queue` and later decide not to run them, you can remove them with | ||
| `dvc exp remove --queue`. | ||
| When you've created experiments to be run in the queue with `dvc queue start` | ||
| and later decide not to run them, you can remove them with | ||
| `dvc queue remove --queued`. |
There was a problem hiding this comment.
@pmrowla should we make this update in other places? The docs still mention exp run --queue a lot.
There was a problem hiding this comment.
The issue here was that dvc exp run --queue was explaining the way the user "created experiments with ..." but dvc queue start is the way the user would "run experiments with ...".
So the original intention was more like
When you've created experiments with
dvc exp run --queueand later decide not to run them, ...
I guess the question is whether or not we can just say "added experiments to the queue" or "queued experiments" instead of providing the commands in these parts of the user guide?
There was a problem hiding this comment.
the question is whether or not we can just say "added experiments to the queue" or "queued experiments" instead of providing the commands
Great Q @pmrowla. I think that in the User Guide (as is the case), we should indeed use descriptive phrasing more, but link to the corresponding sections in the same or other guides.
However, in Refs it's better to link to other commands so it stays self-contained (linking to guides additionally in admonitions is also good, for those who want/need more reading).
Added to best practices.
Link Check Report
All 10 links passed! |
`--run-all` per #3715 (review)
| Starts one or more task queue worker processes. Each worker process will consume | ||
| and execute one queued experiment task at a time in the background, until either | ||
| `dvc queue stop` is used or the queue is empty. | ||
| Starts one or more workers (`--jobs`) to process experiments. Each worker will | ||
| consume and execute one queued experiment tasks at a time in the background, | ||
| until either `dvc queue stop` is used or the queue is empty. |
There was a problem hiding this comment.
- For a possible future follow up: review terms "task queue"/"queued (experiment) task" and "worker process(es)" everywhere. They're probably unnecessary IMO. See ref: document
dvc queue#3715 (review)
| ## Description | ||
|
|
||
| Removes the specified queued or completed experiment tasks from the queue. For | ||
| completed tasks, this will also remove any associated output logs. | ||
| Removes the specified queued or completed experiment `task`(s) from the queue. | ||
| For completed tasks, this will also remove any associated output logs. |
There was a problem hiding this comment.
Do we need to specify "queued or completed"? What other states are there? Only running, I suppose? If so, could phrase like "removes queued exp... Doesn't work with running experiments -- see queue kill".
There was a problem hiding this comment.
There will be other possible states after iterative/dvc#8158
The previous definition of "running" was just "not queued and not completed", but after that PR, RUNNING state in DVC UI specifically means "executing pipeline". And there are other new states to represent setup and cleanup of the experiment.
So for the docs purposes, I think "queued or completed" is still the correct description here. It may also now be better to use something like "active" as the generic "not queued or completed" term instead of "running", so we would get something like "Doesn't work with running active experiments"
There was a problem hiding this comment.
There are also failed experiments that probably should be added to the types of tasks that can be removed.
Active/inactive are new terms but seem intuitive. I'm fine with whatever you both decide.
There was a problem hiding this comment.
It may also now be better to use something like "active" as the generic
I was thinking "in progress" but maybe yours is better.
There are also failed experiments
So again, maybe better to phrase like "Removes non-active experiments from the queue. See queue kill to interrupt active ones" @dberenbaum ?
+ at some point (docs for iterative/dvc#8158?) we could explicitly document the states, maybe grouped by active vs. not.
There was a problem hiding this comment.
So again, maybe better to phrase like "Removes non-active experiments from the queue. See
queue killto interrupt active ones" @dberenbaum ?
Sounds good, thanks!
There was a problem hiding this comment.
Updated in afd08c3.
explicitly document the states, maybe grouped by active vs. not
This is pending. Feel free to make an issue/task/note somewhere. Thanks
| `dvc exp remove --queue`. | ||
| When you've created experiments to be run in the queue with `dvc queue start` | ||
| and later decide not to run them, you can remove them with | ||
| `dvc queue remove --queued`. |
There was a problem hiding this comment.
I thought we agreed that dvc exp remove --queue will stay and we will generally recommend exp over low-level queue commands? Not that important, just surprised to see this suggestion.
There was a problem hiding this comment.
I rolled this back and rephrased a little (6759473) to preserve the original intention (explained by @pmrowla in #3894 (comment)). PTAL
@dberenbaum But it wasn't that clear to me that a) we won't recommend queue start over exp run --all at least until new aliases are decided; and b) why was exp remove --queue changed to queue remove --queued in the original PR then? (Should we roll that back?)
content/docs/user-guide/experiment-management/cleaning-experiments.md
Outdated
Show resolved
Hide resolved
|
Thanks @jorgeorpinel! Would you mind taking a look at my few comments and then hopefully we can merge and call this done? |
| `dvc exp run --queue` and later decide not to run them, you can remove them with | ||
| `dvc exp remove --queue`. | ||
| When you've queued experiments with `dvc exp run --queue` and later decide not | ||
| to run them, you can remove them with `dvc exp remove --queue`. |
There was a problem hiding this comment.
Maybe mention both dvc exp remove and dvc queue remove or keep it generic as suggested above. However, whatever we decide, it should be consistent with the examples below (see line 236 below).
There was a problem hiding this comment.
Rolled back line 236.
keep it generic as suggested above
That seems like a bigger task that applies to many places probably. I'll leave it up to you to create an issue, etc. if needed!
jorgeorpinel
added
the
type: enhancement
Follow up to #3715 (comment)