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

remove specific experiments in queue #2715

Conversation

karajan1001
Copy link
Contributor

@karajan1001 karajan1001 commented Aug 13, 2021

Comment on lines 17 to 18
Deletes one or more experiments, indicated by name (see `dvc exp run`) or
commit SHA (only queued experiments).
Copy link
Contributor

Choose a reason for hiding this comment

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

commit SHA (only queued experiments)

So regular experiments can't be removed by commit SHA @karajan1001 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@karajan1001 can regular commits be removed by SHA? Thanks

This comment was marked as resolved.

Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

Thanks for the update, @karajan1001 !

There's one question ☝️.

And could you add an example of this by the end of the doc? I don't think most people will immediately know what commit SHAs we're referring to.

@jorgeorpinel jorgeorpinel changed the title remove a special experiments in queue remove specific experiments in queue Aug 18, 2021
Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

Improvements to example (will commit).

Comment on lines 17 to 18
Deletes one or more experiments, indicated by name (see `dvc exp run`) or
commit SHA (only queued experiments).

This comment was marked as resolved.

content/docs/command-reference/exp/remove.md Outdated Show resolved Hide resolved
content/docs/command-reference/exp/remove.md Outdated Show resolved Hide resolved
content/docs/command-reference/exp/remove.md Outdated Show resolved Hide resolved
Comment on lines +77 to +95
$ dvc exp show --include-params=train.min_split --no-pager
```

```table
┏━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━┳━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━┳━━━━━━━━━━━━━━━━━┓
┃ Experiment ┃ Created ┃ State ┃ avg_prec ┃ roc_auc ┃ train.min_split ┃
┡━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━╇━━━━━━━━╇━━━━━━━━━━╇━━━━━━━━━╇━━━━━━━━━━━━━━━━━┩
│ workspace │ - │ - │ 0.57553 │ 0.94652 │ 2 │
│ master │ Aug 02, 2021 │ - │ 0.53252 │ 0.9107 │ 2 │
│ └── 5751540 [split32] │ 04:57 PM │ Queued │ - │ - │ 32 │
└───────────────────────┴──────────────┴────────┴──────────┴─────────┴─────────────────┘
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use exp list instead to simplify this example. exp show can be noisy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried on my computer that dvc list could not show queued exps.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, neither dvc exp list nor dvc exp list --all shows the queued experiments. It looks the only way to get the list of queued experiments is via dvc exp show.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK let's keep the exp show table then. Thanks

BTW should exp list print queued exps? Is there an issue in the core repo perhaps?

Copy link
Contributor

Choose a reason for hiding this comment

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

TBH I think the best solution is moving all of this queue functionality into a queue command as suggested in iterative/dvc#5615. I have avoided it because it's a bigger change, but I think it will be needed soon, especially now that we are working on remote execution.

Comment on lines 89 to 113

```dvc
$ dvc exp remove --queue
$ dvc exp show --include-params=train.min_split --no-pager
┏━━━━━━━━━━━━┳━━━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━┳━━━━━━━━━━━━━━━━━┓
┃ Experiment ┃ Created ┃ avg_prec ┃ roc_auc ┃ train.min_split ┃
┡━━━━━━━━━━━━╇━━━━━━━━━━━━━━╇━━━━━━━━━━╇━━━━━━━━━╇━━━━━━━━━━━━━━━━━┩
│ workspace │ - │ 0.57553 │ 0.94652 │ 2 │
│ master │ Aug 02, 2021 │ 0.53252 │ 0.9107 │ 2 │
└────────────┴──────────────┴──────────┴─────────┴─────────────────┘
```
Copy link
Contributor

@jorgeorpinel jorgeorpinel Aug 25, 2021

Choose a reason for hiding this comment

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

Let's delete this and create an Example: remove all experiments in #2722 instead, and throw this in there?

Suggested change
```dvc
$ dvc exp remove --queue
$ dvc exp show --include-params=train.min_split --no-pager
┏━━━━━━━━━━━━┳━━━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━┳━━━━━━━━━━━━━━━━━┓
┃ Experiment ┃ Created ┃ avg_prec ┃ roc_auc ┃ train.min_split ┃
┡━━━━━━━━━━━━╇━━━━━━━━━━━━━━╇━━━━━━━━━━╇━━━━━━━━━╇━━━━━━━━━━━━━━━━━┩
│ workspace │ - │ 0.57553 │ 0.94652 │ 2 │
│ master │ Aug 02, 2021 │ 0.53252 │ 0.9107 │ 2 │
└────────────┴──────────────┴──────────┴─────────┴─────────────────┘
```

Copy link
Contributor

Choose a reason for hiding this comment

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

UPDATE: I'm committing this deletion, should be addressed in #2758 (comment). Thanks

Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

Almost there @karajan1001 I changed some things but there's a question left and something related to your other PR that overlaps. Also the checks are failing, please follow this to get the autoformatting hook in your local env: https://dvc.org/doc/user-guide/contributing/docs or I can fix it if needed. Lmk, thanks!

@jorgeorpinel
Copy link
Contributor

Please merge master @karajan1001 thanks

This was referenced Aug 25, 2021
@karajan1001 karajan1001 force-pushed the remove_a_special_experiments_in_queue branch from 4ab2e36 to a0c323f Compare August 25, 2021 08:23
@karajan1001
Copy link
Contributor Author

Sorry that my wrong operations during rebase deletes all of the changes yesterday, and I had recovered it myself. But might be some mistakes.

Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

Thanks @karajan1001 !

@jorgeorpinel jorgeorpinel merged commit bdb8e31 into iterative:master Sep 1, 2021
@karajan1001 karajan1001 deleted the remove_a_special_experiments_in_queue branch September 2, 2021 02:33
karajan1001 added a commit to karajan1001/dvc.org that referenced this pull request Sep 29, 2021
* remove a special experiments in queue

* Update content/docs/command-reference/exp/remove.md

* Update content/docs/command-reference/exp/remove.md

* Add some example to it

* Recover it

* ref: remove remove --queue example from this PR

* Update content/docs/command-reference/exp/remove.md

* Update content/docs/command-reference/exp/remove.md

* ref: formate exp remove

Co-authored-by: Jorge Orpinel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants