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

[NESTED] guide: Exp implementation details, naming into Overview #3006

Merged
merged 2 commits into from
Nov 17, 2021

Conversation

jorgeorpinel
Copy link
Contributor

@shcheklein shcheklein temporarily deployed to dvc-org-exp-dvc-exp-det-6dklhm November 4, 2021 17:32 Inactive
@jorgeorpinel jorgeorpinel marked this pull request as ready for review November 4, 2021 17:43
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-exp-dvc-exp-det-6dklhm November 4, 2021 17:43 Inactive
Comment on lines +18 to +21
Experiments will have an auto-generated ID like `exp-bfe64` by default. A custom
name can be given instead (using the `--name`/`-n` option of `dvc exp run`).

> ID or name can be used to reference experiments with `dvc exp` subcommands.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iesahin I'm not 100% sure this is right or consistent with the language you've used in other guides. Are exp-abcde called "IDs" or "auto-generated names"? Are the Git hashes called IDs instead? The hashes don't exist for queued experiments so it may be confusing to call those IDs.

We should make sure this terminology makes sense and is consistent 🙂 - let's figure it out here and I'll apply everywhere else.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was using ID and name interchangeably to tell exp-abc123 like names, which can be specified with --name option to dvc exp run. Unfortunately, there are places that we need to tell refs/exps/abc123eeefff333/ IDs as commit IDs.

You're right that we should avoid ID to denote the former, and call the latter commit ID instead. Could this be the naming convention?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"commit ID" may also be confusing though. We already have a term for that I think? I.e. SHA or commit hash.

Maybe we only need "SHA/hash" and "name"? Avoid "ID" completely. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we put "experiment name" to the glossary? When we mention it for the first time, we can link to it.

I'll try to avoid ID completely.

Commit hash = SHA-256 of Git commits
Experiment name = exp-123ab like names in experiments.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have a wiki page about these notes, and review time to time? @jorgeorpinel

Comment on lines +18 to +19
Experiments will have an auto-generated ID like `exp-bfe64` by default. A custom
name can be given instead (using the `--name`/`-n` option of `dvc exp run`).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also (@iesahin), Running Experiments doesn't cover --name so I had to mention the command directly from here. It'd be ideal if there's a section there to link from here instead...

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Nov 4, 2021

Choose a reason for hiding this comment

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

Added a check box about this to #2768 for now.

@shcheklein
Copy link
Member

@jorgeorpinel let's merge this, to then review the main one. It's hard otherwise. Probably not the best case to use nesting (it's better when changes do not overlap much / independent by their nature - not when they both change the same files for the same reason).

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Nov 7, 2021

let's merge this, to then review the main one

Are you sure @shcheklein ? It's going to make reviewing #2909 harder. We could leave this unmerged until the main one is approved instead.

Agree that so far nesting hasn't been more helpful than the complications it introduces. I'll stop trying 😓

@jorgeorpinel jorgeorpinel self-assigned this Nov 7, 2021
@jorgeorpinel jorgeorpinel merged commit dacaf85 into exp/dvc-exps-page Nov 17, 2021
@jorgeorpinel jorgeorpinel deleted the exp/dvc-exp-details branch November 17, 2021 01:19
jorgeorpinel added a commit that referenced this pull request Dec 13, 2021
* guide: add DVC Experiments page and links +
some copy edits

* guide: remove checkpoint related changes

* guide: remove `dvc experiments` long cmd autolinks
per #2901

* guide: move run-cache section back to Exp Mgmt index bottom
per #2909 (review)

* guide: Exp Mgmt/ DVC Exps -> Exps Overview
per #2909 (review)

* guide: clear separation between Exp Mgmt index and Overview page
rel #2909 (comment)

* guide: single guide for Persisting Exps content and
fix links

* guide: begin extracting Exp details from Running to Overview
rel #2909 (comment)

* guide: make ToC entry for Run Cache section
rel #2909 (comment)

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

Co-authored-by: Ivan Shcheklein <[email protected]>

* [NESTED] guide: Exp implementation details, naming into Overview (#3006)

* guide: bring exp implementation details and naming from ref.
per #2909 (review)

* guide: copy edits to exp naming info.

* guide: emphasize dvc exps are not part of Git tree in overview
rel #2909 (review)

* guide: ID->name in dvc exps overview
per #2909 (review)

* guide: ID->name in other exp guides
rel #2909 (review)

* guide: Visualize->Review in exp/overview/basic-workflow
per #2909 (review)

* guide: don't say "cleans the slate" in exp/overview/basic-workflow
per #2909 (review)

* giude: soften params description in exps index
per #2909 (review)

* guide: generalize dvc exps basic workflow

* guide: Properties section in DVC Exps overview page

* guide: exp init section in Exp Overview page

* guide: clarify dvc exp implementation

* guide: expand on Exp Overview motivation
per #2909 (comment)

* guide: direct language in Exp Overview/ workflow intro
per #2909 (comment)

* guide: mention metrics in exp init intro (Exp Overview)
per #2909 (comment)

* guide: intro exp init before giving specific examples of what it does
per #2909 (comment)

* guide: hint forach stages for hybrid exp org pattern
rel. #2909 (comment)

* guide: exp mgmt index copy edits

* guide: mention label-based exp organization
rel. https://docs.google.com/presentation/d/1C_owNoC72GvrpyMGlonHEYJ9I2rl2SLHkZQDMx0eT7A/edit#slide=id.gcb78e52e40_0_635

* guide: hide exp naming section in overview page and
other details
per #2909 (comment)
et al.

* guide: mention `exp init -i` in Overview
per #2909 (comment)

* guide: typo fix
per #2909 (comment)

* ref: exp apply copy edits
per #2909 (review)

* ref: mention init before exp init
per #2909 (review)

* guide: correct info aboug exp init in Exp Overview
per pending comments in #2909 (review)

* ref: link from exp init to corresponding guide

* guide: make exp intro more concrete
per #2909 (comment)

* guide: rewrite exp init section of Exps Overview page
per #2909 (review)

* ref: roll back unrelated ref changes (moved to ref/exp-misc)

* guide: roll back unrelated changes (moved to #3080)

Co-authored-by: Ivan Shcheklein <[email protected]>
iesahin pushed a commit that referenced this pull request Apr 11, 2022
* guide: add DVC Experiments page and links +
some copy edits

* guide: remove checkpoint related changes

* guide: remove `dvc experiments` long cmd autolinks
per #2901

* guide: move run-cache section back to Exp Mgmt index bottom
per #2909 (review)

* guide: Exp Mgmt/ DVC Exps -> Exps Overview
per #2909 (review)

* guide: clear separation between Exp Mgmt index and Overview page
rel #2909 (comment)

* guide: single guide for Persisting Exps content and
fix links

* guide: begin extracting Exp details from Running to Overview
rel #2909 (comment)

* guide: make ToC entry for Run Cache section
rel #2909 (comment)

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

Co-authored-by: Ivan Shcheklein <[email protected]>

* [NESTED] guide: Exp implementation details, naming into Overview (#3006)

* guide: bring exp implementation details and naming from ref.
per #2909 (review)

* guide: copy edits to exp naming info.

* guide: emphasize dvc exps are not part of Git tree in overview
rel #2909 (review)

* guide: ID->name in dvc exps overview
per #2909 (review)

* guide: ID->name in other exp guides
rel #2909 (review)

* guide: Visualize->Review in exp/overview/basic-workflow
per #2909 (review)

* guide: don't say "cleans the slate" in exp/overview/basic-workflow
per #2909 (review)

* giude: soften params description in exps index
per #2909 (review)

* guide: generalize dvc exps basic workflow

* guide: Properties section in DVC Exps overview page

* guide: exp init section in Exp Overview page

* guide: clarify dvc exp implementation

* guide: expand on Exp Overview motivation
per #2909 (comment)

* guide: direct language in Exp Overview/ workflow intro
per #2909 (comment)

* guide: mention metrics in exp init intro (Exp Overview)
per #2909 (comment)

* guide: intro exp init before giving specific examples of what it does
per #2909 (comment)

* guide: hint forach stages for hybrid exp org pattern
rel. #2909 (comment)

* guide: exp mgmt index copy edits

* guide: mention label-based exp organization
rel. https://docs.google.com/presentation/d/1C_owNoC72GvrpyMGlonHEYJ9I2rl2SLHkZQDMx0eT7A/edit#slide=id.gcb78e52e40_0_635

* guide: hide exp naming section in overview page and
other details
per #2909 (comment)
et al.

* guide: mention `exp init -i` in Overview
per #2909 (comment)

* guide: typo fix
per #2909 (comment)

* ref: exp apply copy edits
per #2909 (review)

* ref: mention init before exp init
per #2909 (review)

* guide: correct info aboug exp init in Exp Overview
per pending comments in #2909 (review)

* ref: link from exp init to corresponding guide

* guide: make exp intro more concrete
per #2909 (comment)

* guide: rewrite exp init section of Exps Overview page
per #2909 (review)

* ref: roll back unrelated ref changes (moved to ref/exp-misc)

* guide: roll back unrelated changes (moved to #3080)

Co-authored-by: Ivan Shcheklein <[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.

3 participants