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

guide: revisit Exp Sharing #2908

Merged
merged 81 commits into from
Dec 23, 2021
Merged

guide: revisit Exp Sharing #2908

merged 81 commits into from
Dec 23, 2021

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented Oct 8, 2021

Re-do of #2711 and #2719
Per #2654 (comment)

  • Links to sharing from other docs and other copy edits around the topic
  • Introduce simple diagram for explanation about the necessary remotes
  • Improve explanations (more specific and direct)
  • Remove repeated or unrelated content (covered in other guides) e.g. about dvc exp list
  • Extract short example about looping -> turned to a How to page

PENDING:

jorgeorpinel and others added 30 commits July 21, 2021 08:56
and move run-cache to guide intro (index)
* guide: summarize Sharing Exps intro

* ref: link from exp push/pull to Exp Sharing guide

* Update content/docs/user-guide/experiment-management/sharing-experiments.md

* guide: link from Exp Mgmt index to Sharing

* guide: ~~isolate~~ from link to Exp Sharing
per #2711 (review)

* Update content/docs/user-guide/experiment-management/sharing-experiments.md

Co-authored-by: David de la Iglesia Castro <[email protected]>

* guide: mention only SSH Git URLs support exp sharing
per #2711 (review)

* guide: update dvc remote example in sharing exps

* yarn format some files
per https://app.circleci.com/pipelines/github/iterative/dvc.org/10086/workflows/9b1bf89f-a432-49f2-9a20-72fe77dd4102/jobs/10145

Co-authored-by: David de la Iglesia Castro <[email protected]>
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-guide-exps-shar-7rjper December 17, 2021 07:14 Inactive
@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Dec 17, 2021

explaining --remote and --j doesn't make much sense to me here

It's a good question @shcheklein . I think that @iesahin intended to cover all the possible details in the guide so that we don't have them in the Description of the cmd refs. We probably all agreed to do that here, in fact.

But maybe it's OK to leave out less relevant option explanations (only covered in the Options section of each cmd ref.). This improves the docs focus and maintainability. It's unclear how to define relevant though.

I'm removing them from here and the cmd ref descs. for now (a9988b0). WDYT?

if the page is done right there should be one section around diagram...

The intro currently does that. I'm not trying to completely rewrite the guide. The existing structure also makes sense I think.

We should write about dvc exp list

It's already mentioned here, but mainly covered in Comparing Exps. We have an issue to rewrite that guide though, which structure may be too linear/ disconnected.

examples- up to you... separate subsection "Recipes" or something.

TBH I also just left it because it's already there but didn't find it super relevant either (clever though). I'll remove it for now. And we do have a recipes section, the How To! But there should be a clear need for each step-by-step solution there.

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-guide-exps-shar-7rjper December 17, 2021 07:38 Inactive
@shcheklein
Copy link
Member

I think that @iesahin intended to cover all the possible details in the guide so that we don't have them in the Description of the cmd refs.

To my mind details of the options and their behavior for a specific command should be explained in the cmd ref for that command. UG should be for long sections (remotes) or cross command explanations (experiments overview). Or lengthy explanation how things work.

And that doesn't matter, since -j is still not an essential option, I don't see anything on top of cmd ref options description.

distributed copies of the Git repository, for example on GitHub or GitLab.
Regular <abbr>DVC repository</abbr> versions are typically synchronized among
team members per regular Git workflow. [DVC Experiments] are directly woven into
this commit history. But they also intend to avoid cluttering shared repos, so
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I'm not sure I understand this

DVC Experiments] are directly woven into this commit history.

Let's try to write is simple and very explicit.

regular Git workflow, namely creating Git commits, pushing them to Github or from Github.

It means that experiments become part of the Git history, they are visible in Git, etc. As we mentioned this is not idea since doesn't scale well, creates mess in Git history etc. DVC experiments still rely on Git and Github, but they are not directly attached ... so won't visible ... etc ..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's try to write is simple and very explicit.

Simplified in 0fb52cb.

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Dec 22, 2021

Choose a reason for hiding this comment

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

I think the rest of your suggestions are great but do we want to elaborate so much on general motivation for DVC Experiments in this guide @shcheklein ? Maybe we should separately review that this is clearly included in the Overview page and/or the Running Exps page. (Added check box to PR desc.)

For now the only mention of motivation is "to avoid cluttering everyone's copies of the repo, by default experiments will only exist in the local environment where they were created."

@shcheklein
Copy link
Member

. The existing structure also makes sense I think.

Yes, but it's not ideal. Those small subsections could be removed completely - just one paragraph after the diagram is enough . Hey, you have push/pull/list to share experiments this way. Here are the links to read the details (links to the cmd ref).

Thus the whole purpose of this page is the diagram and text around it - that is the most valuable part. We can write more and more details, caveats that are general for that workflow, write advanced stuff how to see git index, etc.

Also, diagram might include exp pull, exp push?

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-guide-exps-shar-7rjper December 22, 2021 03:12 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-guide-exps-shar-7rjper December 22, 2021 03:37 Inactive
@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Dec 22, 2021

The existing structure also makes sense I think.

Yes, but it's not ideal. Those small subsections could be removed completely

I agree with you @shcheklein. I argued for that from the original PR but we decided to merge something. And this PR is about revisiting that guide so it makes sense to bring it up again. Buuuttt that's actually a task in #2911. I actually intended to only improve the guide with it's current structure here. If you don't think this is valuable enough then I can plan to repurpose the PR, but I think merging the work so far would be a good first step (lots of improvements).

diagram might include exp pull, exp push?

Added pull in d6de4f5.

@iesahin
Copy link
Contributor

iesahin commented Dec 22, 2021

I tend to diverge from you about the reading habits of users, so I'm including more headers the readers could skim. Unifying those sections might produce a more beautiful document but the users will spend a longer time finding the information they need.

@shcheklein
Copy link
Member

I think we diverge here primarily on the content and the focus in the document, not reading habits. For me removing subsections make sense since I don't see much content in them that should stay - we don't it in the user guide in this form (e.g. -j - why only this option? what is the benefit to repeat it this way, etc).

@jorgeorpinel jorgeorpinel merged commit 3fab9cd into master Dec 23, 2021
@jorgeorpinel jorgeorpinel deleted the guide/exps-sharing branch December 23, 2021 06:07
@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Dec 23, 2021

TBH in my answer I was thinking more of https://dvc.org/doc/user-guide/experiment-management/comparing-experiments, not the one (sharing exps), sorry for the confusion.

As for reading habits, FWIW personally I usually try to Ctrl+F key words first. Reading the ToC or headers is a probable next strategy but I expect that kind of structure more in the most technical docs. For a guides I'd assume that to really get it you need to read the whole thing or at least a good chunk at the beginning before skipping ahead.

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Dec 23, 2021

p.s. but I have to admit I'm still not sure when we want guides to be more technical vs. refs (or in what sense).

jorgeorpinel added a commit that referenced this pull request Dec 23, 2021
* ref: bring changes from #2909

* guide: bring changes from #2909

* exp: pull some changes from #2908

* ref: better explain exp apply behavior explicitly
rel. #3080 (comment)

* ref: exp apply destroys any canges
per #3080 (comment)

* ref: clarify that exp branch makes 1+ commits in the exp branch
rel. #3080 (comment)

* ref: link exp apply -> exp branch
per #3080 (comment)

* ref: clarify (again) what applies does specifically
per #3080 (review)

* ref: document intended apply behavior on conflicting changes
per #3080 (review)
iesahin pushed a commit that referenced this pull request Apr 11, 2022
* guide: split Experiments (index) into sub-pages

* case: keep Persistent Exps in basic page

* cases: keep Run-cache in basic Exps page

* guide: edit Exp Mgmt index (intro)

* guide: edit basic Exps page inc. persisting them
and move run-cache to guide intro (index)

* guide: rename DVC Exps, remove Org Exps page

* guide: bash -> dvc in EM/Checkpoints

* guide: fix exps link

* guide: summarize Sharing Exps intro

* ref: link from exp push/pull to Exp Sharing guide

* Update content/docs/user-guide/experiment-management/sharing-experiments.md

* guide: rename Exp Sharing sections

* guide: summarize Exp Sharing examples

* guide: link from Exp Mgmt index to Sharing

* guide: ~~isolate~~ from link to Exp Sharing
per #2711 (review)

* Update content/docs/user-guide/experiment-management/sharing-experiments.md

Co-authored-by: David de la Iglesia Castro <[email protected]>

* guide: mention only SSH Git URLs support exp sharing
per #2711 (review)

* guide: update dvc remote example in sharing exps

* yarn format some files
per https://app.circleci.com/pipelines/github/iterative/dvc.org/10086/workflows/9b1bf89f-a432-49f2-9a20-72fe77dd4102/jobs/10145

* guide: consolidate Exp Sharing intro (#2711)

* guide: summarize Sharing Exps intro

* ref: link from exp push/pull to Exp Sharing guide

* Update content/docs/user-guide/experiment-management/sharing-experiments.md

* guide: link from Exp Mgmt index to Sharing

* guide: ~~isolate~~ from link to Exp Sharing
per #2711 (review)

* Update content/docs/user-guide/experiment-management/sharing-experiments.md

Co-authored-by: David de la Iglesia Castro <[email protected]>

* guide: mention only SSH Git URLs support exp sharing
per #2711 (review)

* guide: update dvc remote example in sharing exps

* yarn format some files
per https://app.circleci.com/pipelines/github/iterative/dvc.org/10086/workflows/9b1bf89f-a432-49f2-9a20-72fe77dd4102/jobs/10145

Co-authored-by: David de la Iglesia Castro <[email protected]>

* prettier sharing-experiments.md

* Update content/docs/user-guide/experiment-management/sharing-experiments.md

Co-authored-by: Casper da Costa-Luis <[email protected]>

* guide: roll back wrong files

* guide: roll back Exp Mgmt index...

* guide: link to Sharing Exps from index

* guide: Listing exps on remotes
per #2908 (review)

* guide: don't mention Git here...
per #2908 (review)

* guide: clarify that git is needed for exps and sharing
per #2908 (review)

* guide: clarify note on Git requirement for DVC Exps
per #2908 (review)

* guide: simplify Sharing Exps intro (rel Git)
per #2908 (review)

* guide: rename exp list -r section

* copy edit

* cases: simplify note about requiring Git
per #2908 (review)

* guide: emoji for example in Sharing Exps
per #2908 (comment)

* guide: clarify note about Git-DVC repo required for Exps
per #2908 (review)

* Update content/docs/user-guide/experiment-management/sharing-experiments.md

* guide: another example emoji en Sharing Exps

* Restyled by prettier (#2972)

Co-authored-by: Restyled.io <[email protected]>

* guide: list exps in Comparing guide, linked from Sharing
per #2908 (comment)

* guide: address feedback from
#2908 (review) and below

* guide: rephrase Git history exps org
per #2908 (review)

* guide:address Exp sharing feedback from
from #2908 (review) and below

* guide: update Git remote auth limitation wording
per #2908 (comment)

* guide: more copy edits on Exp Sharing and Comparing

* guide: clarify `exp list` remote info
per #2908 (review)

* guide: un0hide exp sharing details
per #2908 (review)

* guide: move multi-exp share example to how-to
per #2908 (review)

* guide: simplify Exp Sharing intro, add diagram
per should be focusing more on explaining (in simple terms, with diagrams) how it works

* guide: fix SSH URLS link in Exp Sharing...

* exp: roll back unrelated changes

* guide: Git -> Git remote
per #2908 (review)

* guide: improve Sharing exp intro
per #2908 (review)

* exp push/pull: remove --remote and --jobs details from guide and ref descs.
rel. #2908 (comment)

* guide: remove Sharing Exps example
per #2908 (comment)

* guide: simplify Sharing Exps intro
per #2908 (review)

* guide: add exp pull to diagram in Sharing Exps
per #2908 (comment)

Co-authored-by: David de la Iglesia Castro <[email protected]>
Co-authored-by: Casper da Costa-Luis <[email protected]>
Co-authored-by: restyled-io[bot] <32688539+restyled-io[bot]@users.noreply.github.com>
Co-authored-by: Restyled.io <[email protected]>
iesahin pushed a commit that referenced this pull request Apr 11, 2022
* ref: bring changes from #2909

* guide: bring changes from #2909

* exp: pull some changes from #2908

* ref: better explain exp apply behavior explicitly
rel. #3080 (comment)

* ref: exp apply destroys any canges
per #3080 (comment)

* ref: clarify that exp branch makes 1+ commits in the exp branch
rel. #3080 (comment)

* ref: link exp apply -> exp branch
per #3080 (comment)

* ref: clarify (again) what applies does specifically
per #3080 (review)

* ref: document intended apply behavior on conflicting changes
per #3080 (review)
@jorgeorpinel jorgeorpinel added type: enhancement Something is not clear, small updates, improvement suggestions A: docs Area: user documentation (gatsby-theme-iterative) C: guide Content of /doc/user-guide labels Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: docs Area: user documentation (gatsby-theme-iterative) C: guide Content of /doc/user-guide type: enhancement Something is not clear, small updates, improvement suggestions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants