Remove a special queued experiments #6393
Conversation
fix iterative#6157 1. dvc exp remove not accept queued experiments name 2. add some tests for this feature
|
So one other thing that will be needed is removing queued experiments by revision. For regular experiments we only support removing them by ref (or shortened name), since the operation to remove a finished experiment is a git ref deletion, and using commit SHA's here would be ambiguous. But for queued (stashed) experiments, they will only be named if the user provided one themselves with |
Yes, there comes another question should the committed ones also accept a revision name? It is natural for uses to extend the usage of queued ones to a committed one. |
No, because the actual operations for removing a queued experiment and a regular one are two different things. Since removing a regular experiment is a git ref deletion, specifying a commit SHA is ambiguous - it's possible for multiple experiment refs to point to a single git SHA, so it is better for us to require the user to be explicit here (and use the ref/name). Removing queued experiments is not deleting a git ref, it's dropping a git SHA from a list of stashed git SHAs. |
1. Extract remove experiments to a new file. 2. revision can be used to remove special queued experiment
dvc/repo/experiments/remove.py
Outdated
| if stash_index is None: | ||
| raise InvalidArgumentError( | ||
| f"'{ref_or_rev}' is neither a valid experiment reference" | ||
| " nor a queued experiment revision" |
There was a problem hiding this comment.
Quite weird here, because the asymmetric in committed and queued experiments.
There was a problem hiding this comment.
I would separate this into two methods, one for removing a complete experiment ref (only by ref name) and one for removing stash entries (by either name or revision).
There was a problem hiding this comment.
But without a flag, we can not know if the input is a ref or a revision. Some ref might use a shortcut name for example.
epoch_1_layers3 - > ep1la3 hard to distinguish them.
There was a problem hiding this comment.
Right, you would start by calling
_remove_exp_ref(name).
If that fails because an exact exp ref match for name does not exist, then you would try the stash remove function. Which also needs to try for an exact name match first, and then handling it as git rev second.
There was a problem hiding this comment.
Basically you can parse the list of names that we get at the start into two separate lists before trying to remove anything at all. One list that matches exact refs, and a second list that does not match exact refs. Everything in the second list is either a stash entry (by name or rev) or is invalid entirely.
dvc/repo/experiments/remove.py
Outdated
| print("*" * 100) | ||
| for rev, ref_info in stash_ref_infos.items(): | ||
| print(rev, ref_info) |
There was a problem hiding this comment.
Excuse me, How to set an auto-commit for the VSCODE, always get some changes not committed or not updated.
dvc/repo/experiments/remove.py
Outdated
| if stash_index is None: | ||
| raise InvalidArgumentError( | ||
| f"'{ref_or_rev}' is neither a valid experiment reference" | ||
| " nor a queued experiment revision" |
There was a problem hiding this comment.
I would separate this into two methods, one for removing a complete experiment ref (only by ref name) and one for removing stash entries (by either name or revision).
|
Some example to this |
| removed += len(ref_infos) | ||
| remained = _remove_commited_exps(repo, exp_names) | ||
| remained = _remove_queued_exps(repo, remained) | ||
| if remained: |
There was a problem hiding this comment.
A behavior change here, because we had already removed all of the matched experiments, raise an exception here is needless.
|
Better tests cases.
We still need to raise Exception in a mixed committed and non exist case. Because of only by this the return values of |
* 'python310' of github.com:skshetry/dvc: Unpin networkx Remove a special queued experiments (iterative#6393) build(deps): bump google-cloud-storage from 1.41.1 to 1.42.0 (iterative#6415) Push running checkpoint to remote (iterative#6332)
fix #6157
❗ I have followed the Contributing to DVC checklist.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏