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: clarifications to exp run #2368

Merged
merged 13 commits into from
Apr 13, 2021
Merged

ref: clarifications to exp run #2368

merged 13 commits into from
Apr 13, 2021

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented Apr 10, 2021

@shcheklein shcheklein temporarily deployed to dvc-org-ref-exp-run-a7mphvuvua April 10, 2021 17:19 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-ref-exp-run-a7mphvuvua April 10, 2021 17:31 Inactive
@jorgeorpinel jorgeorpinel requested a review from pmrowla April 10, 2021 18:44
The results of the last experiment can be seen in the <abbr>workspace</abbr>. To
display and compare your experiments, use `dvc exp show` or `dvc exp diff`. Use
The results of the last experiment can be seen in the workspace. To display and
compare your experiments, use `dvc exp show` or `dvc exp diff`. Use
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should document that all of the other DVC diff commands and plots commands can also be used with experiments here

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'd wait to see if all the show and diff commands get merged 🙂

Copy link
Contributor

@pmrowla pmrowla Apr 12, 2021

Choose a reason for hiding this comment

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

Unification of the show and diff commands is something that probably won't happen for a decent amount of time, and I brought it up because I have seen questions from users in discord asking about how to use plots with experiments.

either way it's not a big deal I guess

This comment was marked as resolved.

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Apr 12, 2021

Choose a reason for hiding this comment

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

DVC diff commands and plots commands can also be used with experiments here

Wait. But we mean more like show and compare MANY/ALL experiments here. metrics/params diff only take 2 revs and in those cases exp diff is prob better/easier anyway.

This only really applies to plots diff as of now, I think? Mentioned just that in 29004e6 for now.

Copy link
Contributor

@dberenbaum dberenbaum Apr 12, 2021

Choose a reason for hiding this comment

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

I think that approach makes sense @jorgeorpinel.

  • Alternatively (or additionally), we could say something like "any dvc command that accepts Git revisions also accepts experiment names."

jorgeorpinel added a commit that referenced this pull request Apr 12, 2021
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-ref-exp-run-a7mphvuvua April 12, 2021 06:34 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-ref-exp-run-a7mphvuvua April 12, 2021 06:36 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-ref-exp-run-a7mphvuvua April 12, 2021 06:37 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-ref-exp-run-a7mphvuvua April 12, 2021 06:40 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-ref-exp-run-a7mphvuvua April 12, 2021 06:40 Inactive
@jorgeorpinel jorgeorpinel marked this pull request as ready for review April 12, 2021 06:41
@jorgeorpinel jorgeorpinel requested a review from pmrowla April 12, 2021 06:43
@jorgeorpinel jorgeorpinel requested a review from dberenbaum April 12, 2021 06:43
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-ref-exp-run-a7mphvuvua April 12, 2021 06:46 Inactive
Copy link
Contributor

@dberenbaum dberenbaum left a comment

Choose a reason for hiding this comment

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

Agree that "in the background" doesn't seem quite right since temp experiments are blocking once they are run. Otherwise looks good, and I can take one more look once that issue is resolved.

content/docs/command-reference/exp/run.md Outdated Show resolved Hide resolved
content/docs/command-reference/exp/run.md Outdated Show resolved Hide resolved
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-ref-exp-run-a7mphvuvua April 12, 2021 18:54 Inactive
@jorgeorpinel jorgeorpinel had a problem deploying to dvc-org-ref-exp-run-a7mphvuvua April 12, 2021 18:58 Failure
Copy link
Contributor Author

@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.

Couple more Qs @pmrowla 👇 Thanks!

Comment on lines 113 to 114
Use `dvc exp run --run-all` to process the queue. This is done outside your
<abbr>workspace</abbr> (in a temporary dir in `.dvc/tmp/exps`), so you can
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are exp queues always processed outside the workspace, even with -j 1? And why

Copy link
Contributor

Choose a reason for hiding this comment

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

To me, "queueing" means setting up an experiment for future execution. To do that, I would expect to add to the queue, then keep making changes to my workspace without impacting the experiment that I already queued. It seems like this workflow would always require processing outside the workspace, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this workflow would always require processing outside the workspace

At exp run --run-all it could checkout the workspace locally at each queue time, overwriting whatever you have there and forcing you to wait for it to finish.

Copy link
Contributor

Choose a reason for hiding this comment

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

overwriting whatever you have there

It could do that, but overwriting the workspace isn't ideal, right? I know the current solution isn't perfect either, but avoiding having to overwrite the workspace seems like a big reason why exp queues are always processed outside the workspace (in addition to consistency with parallel execution and possibly remote scenarios in the future).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah, I agree! I'm just saying it's not necessarily obvious, so we should be explicit about the behavior I think. I guess you were just answering the Q (with a yes). Updated a bit in d8acecb anyway. LGTM!

content/docs/command-reference/exp/run.md Show resolved Hide resolved
@jorgeorpinel jorgeorpinel requested a review from dberenbaum April 12, 2021 19:03
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-ref-exp-run-a7mphvuvua April 12, 2021 20:42 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-ref-exp-run-a7mphvuvua April 12, 2021 20:44 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-ref-exp-run-a7mphvuvua April 12, 2021 22:50 Inactive
@jorgeorpinel jorgeorpinel merged commit a6dce61 into master Apr 13, 2021
@jorgeorpinel jorgeorpinel deleted the ref/exp/run branch April 24, 2021 07:34
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