-
Notifications
You must be signed in to change notification settings - Fork 394
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
experiment-management: remove warning about no parameter validation #2824
experiment-management: remove warning about no parameter validation #2824
Conversation
Covers feature change iterative/dvc#6521. Parameters will now be validated. Related issue iterative/dvc#5477
This simply removes the warning about lack of parameter validation at the moment. |
Do you mind marking this as a draft until iterative/dvc#5477 is merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it's all we need here actually so I'll make it a regular PR and just label it. Thanks @mattlbeck
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far so good but we need a few other changes:
- The text in Updating experiment parameters on-the-fly should say something like "-S option takes an existing parameter name and"...
- Optinally/Additionally, the sample command block under that same text could have
dvc param list
and it's output (showingmodel.learning_rate
among 1-2 other params) before theexp run --set-param model.learning_rate
command). - Corresponding update to https://dvc.org/doc/command-reference/exp/run#options
- Same in https://dvc.org/doc/command-reference/exp/run#example-modify-parameters-on-the-fly
Thanks
@jorgeorpinel unless I am missing something |
content/docs/user-guide/experiment-management/running-experiments.md
Outdated
Show resolved
Hide resolved
content/docs/user-guide/experiment-management/running-experiments.md
Outdated
Show resolved
Hide resolved
Yes @mattlbeck sorry, I made up a command haha. I applied your idea. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah no, I see everything is addressed. Thanks again @mattlbeck !
I merged #2944 but this is still open 🤔 and now has more conflicts... |
I think Restyled is having some issues (again)... After merging it to master, this PR showed conflicts with the formatted bits (should've been closed by that). And now that I solved the conflicts (used |
My master merge wasn't correct -- left an accidental trailing space in there. The Restyled PR changed it back to the already merged version in Now I fixed the formatting here and there are no changes in this PR... 🤷 merging! |
Per iterative/dvc#6521
Covers feature change iterative/dvc#6521. Parameters will now be validated.
Related issue iterative/dvc#5477