-
Notifications
You must be signed in to change notification settings - Fork 29
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
Persisting experiments from table: share as a branch and commit and share #2855
Comments
Thanks for having a look at this! |
Share as a branchWhat the command does (happy path) sequentially:
Looking at the code I just realised that these steps are not outlined to the user. Only the CLI output is shown. Would some more messaging help? I.e "Creating branch..."
Looking closer at this image: The information you are displaying does not come from this extension. It comes from VS Code's built-in Git extension. Under these circumstances I would first try this button: |
Commit and shareFrom your screenshots it looks like you are trying to commit and share an experiment from the previous commit. It seems that it is not possible to apply an experiment from a previous commit onto the workspace using DVC. I'll do some more investigation before raising a bug there 👍🏻. Edit: iterative/dvc#8481 is already open. |
What commit and share does (happy path) sequentially: Applies experiment to workspace. Order of operation for share as branch: #2855 (comment) |
[Q] @dberenbaum do you think we should limit these actions to the current commit or keep them available for any of the experiments displayed? |
How much work does that take? @pmrowla @karajan1001 How much work would it take to address iterative/dvc#8481? I would rather do the latter, but if it's much easier to disable in VS Code in the short term, then it makes sense. |
@dberenbaum do we know why we have this limitation? (what is the core reason). I'm not sure I understand what is the difference between commits tbh. |
Since the exp ref stashes changes from some baseline commit, it's safe to apply that stash on top of the commit from which it originated. If it is applied on top of some other commit, it could create conflicts or maybe even just unexpected results. When experiments was first implemented, it was sufficient to apply on top of the baseline commit, and applying on top of other commits might have been considered too dangerous. Now that we see it used more, I think the pros outweigh the cons, especially since we should be able to rollback in case of conflicts. That does complicate things for VS Code, though, since there's no guarantee the apply will work (the same could be true for a dirty workspace even on top of the baseline commit). |
It's not a stash though, right? It's more or less a proper hidden commit? In this sense, there should be not difference to which ref to hard reset to to my mind. It's not like applying on top, it should be more like:
wdyt? |
Hey @shcheklein, I moved this discussion back into iterative/dvc#8481. In the short term, should we limit these actions to the current commit since it seems like there's still product questions on how Edit: sounds like this is not an easy fix on the vs code side, so limiting to the current commit is not necessarily the best approach. |
about 1-2 days? |
@karajan1001 @dberenbaum any thought / further actions on this? |
Nothing planned yet since @karajan1001 still has a few queue bugs he's working on AFAIK. I think there's still some product questions we can address in the meantime in iterative/dvc#8481. |
Should be fixed by iterative/dvc#8481. More than likely that this functionality will either:
Closing for now. |
Context
Running experiments from workshop. Find a parameter that impacts model performance. Testing persistance UI table features. In table , access to context menu, and then click in share as a branch
Share as a branch
Pop up menu appears and
git checkout
is doneISSUE : Inconsistency of branches from CLI and VSCode. CLI tells me that Im in that branch, but I can´t find it . Unclear if I have to publish first or if the extension does it for me . I´m probably missing something . Can you help ?
The text was updated successfully, but these errors were encountered: