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

Add validating webhooks for early stopping experiment #1680

Closed
tenzen-y opened this issue Sep 25, 2021 · 9 comments · Fixed by #1709
Closed

Add validating webhooks for early stopping experiment #1680

tenzen-y opened this issue Sep 25, 2021 · 9 comments · Fixed by #1709
Assignees

Comments

@tenzen-y
Copy link
Member

/kind feature

Describe the solution you'd like
[A clear and concise description of what you want to happen.]

Currently, there are no validating webhooks for early stopping, so users can not notice wrong parameters until to start trial Pod.
So, I'd like to add validating webhooks for the following parameters.

  • spec.earlyStopping.algorithmName
  • spec.earlyStopping.algorithmSettings

What do you think?

/assign

@tenzen-y
Copy link
Member Author

Does it sound good ? @andreyvelich @gaocegege @johnugeorge

@andreyvelich
Copy link
Member

Thank you for creating this!
Do you mean similar to ValidateAlgorithm ?

@tenzen-y
Copy link
Member Author

Thank you for your comment!
Yes, I consider it is better to implement it using GetEarlyStoppingConfigData.

@andreyvelich
Copy link
Member

Sounds good!

@tenzen-y
Copy link
Member Author

I take this Issue.

@tenzen-y
Copy link
Member Author

tenzen-y commented Sep 29, 2021

@andreyvelich
I found there are no unit tests for GetFooConfigData(ex. GetSuggestionConfigData) in katibconfig when I was implementing unit tests for this Issue. Are they being tested anywhere?

@tenzen-y
Copy link
Member Author

I'd like to create PR to add unit tests for GetFooConfigData (ex. GetSuggestionConfigData) before resolving this issue.

@andreyvelich
Copy link
Member

andreyvelich commented Sep 30, 2021

You are right @tenzen-y.
Currently, only composer tests GetSuggestionConfig, but it doesn't cover all test-cases.

@tenzen-y
Copy link
Member Author

tenzen-y commented Sep 30, 2021

Thank you for your answer! @andreyvelich
First, I'd like to implement unit tests for GetFooConfigData.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants