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

[GSoC] Added DistributionType to Experiment API #2377

Merged

Conversation

shashank-iitbhu
Copy link
Contributor

What this PR does / why we need it:

  • Added DistributionType to ParameterSpec in pkg/apis/controller/experiments/v1beta1/experiment_types.go.
  • Defined DistributionType.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2374

Checklist:

  • Docs included if any changes are user facing

@shashank-iitbhu
Copy link
Contributor Author

shashank-iitbhu commented Jul 8, 2024

ParameterType and DistributionType can be made exclusive using this logic

// Check that only one of ParameterType or DistributionType is specified
        if (param.ParameterType != "" && param.DistributionType != "") ||
            (param.ParameterType == "" && param.DistributionType == "") {
            allErrs = append(allErrs, field.Invalid(parametersPath.Index(i).Child("parameterType"),
                param.ParameterType, "either parameterType or distributionType must be specified, but not both"))
        }

at

func (g *DefaultValidator) validateParameters(parameters []experimentsv1beta1.ParameterSpec) field.ErrorList {
var allErrs field.ErrorList
for i, param := range parameters {

cc @andreyvelich @tenzen-y

@shashank-iitbhu
Copy link
Contributor Author

/area gsoc

DistributionLogUniform Distribution = "logUniform"
DistributionNormal Distribution = "normal"
DistributionLogNormal Distribution = "logNormal"
DistributionCategorical Distribution = "categorical"
Copy link
Member

Choose a reason for hiding this comment

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

We should remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, removed Categorical from Distribution type.

Signed-off-by: Shashank Mittal <[email protected]>

modified feasibleSpace

Removed Categorical from Distribution
Copy link
Member

@tenzen-y tenzen-y left a comment

Choose a reason for hiding this comment

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

Thank you!
/lgtm
/approve

Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tenzen-y

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 6a17c3e into kubeflow:master Jul 31, 2024
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GSOC] Project 8: Support various Parameter Distribution in Katib
3 participants