[GSOC] `optuna` suggestion service logic update #2446

shashank-iitbhu · 2024-10-20T19:47:30Z

What this PR does / why we need it:

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

google-oss-prow · 2024-10-20T19:47:37Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign gaocegege for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

github-actions · 2025-01-18T20:05:09Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

mahdikhashan · 2025-01-29T14:51:30Z

I started reviewing this pr

cc: @andreyvelich

mahdikhashan · 2025-01-30T07:53:09Z

/rerun-all

mahdikhashan · 2025-01-30T07:54:34Z

@shashank-iitbhu would you please rerun the ci?

shashank-iitbhu · 2025-01-30T10:48:33Z

@mahdikhashan still working on this PR, need some time. This is based on #2412 , so it makes sense to merge this once #2412 is merged.

mahdikhashan · 2025-01-30T11:44:48Z

@mahdikhashan still working on this PR, need some time. This is based on #2412 , so it makes sense to merge this once #2412 is merged.

ok, take your time, are there any place which I can offer you help?

Electronic-Waste · 2025-01-31T03:39:06Z

Hi @shashank-iitbhu, #2412 has been merged. We can work on this now!

But before that, could you please rebase your PR and resolve the errors in CI?

mahdikhashan · 2025-02-03T08:16:21Z

Hi @shashank-iitbhu, #2412 has been merged. We can work on this now!

But before that, could you please rebase your PR and resolve the errors in CI?

@shashank-iitbhu shall we start reviewing it?

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

Co-authored-by: Yuki Iwai <yuki.iwai.tz@gmail.com> Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

shashank-iitbhu · 2025-02-03T12:47:57Z

Hi @shashank-iitbhu, #2412 has been merged. We can work on this now!
But before that, could you please rebase your PR and resolve the errors in CI?

@shashank-iitbhu shall we start reviewing it?

Sure, I have rebased this PR also, I will let you know once this is ready for review.

mahdikhashan · 2025-02-03T12:51:28Z

Hi @shashank-iitbhu, #2412 has been merged. We can work on this now!
But before that, could you please rebase your PR and resolve the errors in CI?

@shashank-iitbhu shall we start reviewing it?

Sure, I have rebased this PR also, I will let you know once this is ready for review.

thank you! feel free to ping me when you're ready. btw, we can also do a progressive review. would it work for you to review the existing code before finalizing it?

andreyvelich · 2025-02-03T13:13:09Z

Hi @shashank-iitbhu, we have time until this Wednesday to merge this PR before we cut Katib RC.0.
Do you have enough time to finish it ?

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

shashank-iitbhu · 2025-02-03T20:03:28Z

test/unit/v1beta1/suggestion/test_optuna_service.py

@@ -50,7 +50,7 @@ def setup_method(self):
            ],
            ["cmaes", {"restart_strategy": "ipop", "sigma": "2", "random_state": "71"}],
            ["random", {"random_state": "71"}],
-            ["grid", {"random_state": "71"}],
+            # ["grid", {"random_state": "71"}],


@mahdikhashan @andreyvelich The unit tests are failing for this grid algorithm setting for param-11 and param-12 as here I am not specifying the step in parameters for param_type DOUBLE.
An Exception Error keeps raising due to call to convert_to_combinations method

katib/pkg/suggestion/v1beta1/internal/search_space.py

Lines 57 to 62 in 3e736dc

elif parameter.type == DOUBLE:

if parameter.step == "" or parameter.step is None:

raise Exception(

"Param {} step is nil; For discrete search space, all parameters "

"must include step".format(parameter.name)

)

katib/pkg/suggestion/v1beta1/optuna/base_service.py

Lines 56 to 59 in 3e736dc

elif self.algorithm_name == "grid":

combinations = HyperParameterSearchSpace.convert_to_combinations(

self.search_space

)

I tried to explicitly check for nil and set parameter.step to default 1 but then getting this error while running the unit tests.
E AssertionError: assert <StatusCode.DEADLINE_EXCEEDED: (4, 'deadline exceeded')> == <StatusCode.OK: (0, 'ok')>
How to deal with this?

I can look into this tomorrow in early morning, hope its fine with you.

it seems, to my current understanding of Optuna docs, that for grid algo, we need to have step defined. I'll investigate further with setting step and the second error.

I ran a deubg session and it seems that not all params are available in the search space - there should be param 10 and 11 - (i commented out 12s)

I tried to explicitly check for nil and set parameter.step to default 1 but then getting this error while running the unit tests. E AssertionError: assert <StatusCode.DEADLINE_EXCEEDED: (4, 'deadline exceeded')> == <StatusCode.OK: (0, 'ok')> How to deal with this?

I could not figure out where we can change to handle it - will try to dig deeper later.

shashank-iitbhu · 2025-02-03T20:18:20Z

Hi @shashank-iitbhu, we have time until this Wednesday to merge this PR before we cut Katib RC.0. Do you have enough time to finish it ?

Hey @andreyvelich, I just pushed the changes for the unit tests and e2e tests. I’m facing an issue while testing the grid algorithm setting. Also, one more thing to note: Optuna does not directly support the normal distribution. Once the above issue is resolved, I believe we’re good to merge this.

https://optuna.readthedocs.io/en/stable/reference/distributions.html
cc @mahdikhashan

pkg/suggestion/v1beta1/hyperopt/base_service.py

pkg/suggestion/v1beta1/optuna/base_service.py

mahdikhashan · 2025-02-04T10:43:19Z

test/unit/v1beta1/suggestion/test_optuna_service.py

@@ -50,7 +50,7 @@ def setup_method(self):
            ],
            ["cmaes", {"restart_strategy": "ipop", "sigma": "2", "random_state": "71"}],
            ["random", {"random_state": "71"}],
-            ["grid", {"random_state": "71"}],
+            # ["grid", {"random_state": "71"}],


it seems, to my current understanding of Optuna docs, that for grid algo, we need to have step defined. I'll investigate further with setting step and the second error.

mahdikhashan · 2025-02-04T12:31:37Z

test/unit/v1beta1/suggestion/test_optuna_service.py

@@ -50,7 +50,7 @@ def setup_method(self):
            ],
            ["cmaes", {"restart_strategy": "ipop", "sigma": "2", "random_state": "71"}],
            ["random", {"random_state": "71"}],
-            ["grid", {"random_state": "71"}],
+            # ["grid", {"random_state": "71"}],


I ran a deubg session and it seems that not all params are available in the search space - there should be param 10 and 11 - (i commented out 12s)

Electronic-Waste · 2025-02-04T15:11:01Z

Thanks @shashank-iitbhu! And also thanks for reviewing this @mahdikhashan!

I have no extra comments on this PR. As for the normal distribution, does optuna have plans to support it? If the answer is yes, I would suggest that we could create a separate issue tracking this. If no, we could declare in the user doc that optuna algorithm only supports uniform and logUniform.

WDYT @kubeflow/wg-automl-leads @shashank-iitbhu @mahdikhashan @helenxie-bit

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

andreyvelich · 2025-02-05T14:29:24Z

pkg/suggestion/v1beta1/optuna/base_service.py

+                elif param.distribution == api_pb2.LOG_UNIFORM:
+                    # Log-uniform integer distribution: used for exponentially varying integers.
+                    search_space[param.name] = optuna.distributions.IntDistribution(
+                        low=max(1, int(param.min)),


Would this always be equal to 1 since values are always int here ?

Not always. The condition ensures that low is at least 1, which is required for log distributions. It prevents invalid cases where param.min could be 0 or negative.
As specified in the optuna documentation, If log is True, low must be larger than or equal to 1.

andreyvelich · 2025-02-05T15:07:52Z

pkg/suggestion/v1beta1/optuna/base_service.py

+                        step=None,
+                    )
+
+            elif param.type in {CATEGORICAL, DISCRETE}:


Should we use array here ?

Suggested change

elif param.type in {CATEGORICAL, DISCRETE}:

elif param.type in [CATEGORICAL, DISCRETE]:

andreyvelich · 2025-02-05T15:08:53Z

examples/v1beta1/hp-tuning/optuna-distribution.yaml

+        min: "0.001"
+        max: "1"
+        step: "0.1"
+        distribution: uniform


Please add one parameters for Float and LogUniform

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

google-oss-prow bot requested review from anencore94 and Electronic-Waste October 20, 2024 19:47

google-oss-prow bot added the size/XL label Oct 20, 2024

shashank-iitbhu marked this pull request as draft October 20, 2024 19:48

google-oss-prow bot added the do-not-merge/work-in-progress label Oct 20, 2024

github-actions bot added the lifecycle/stale label Jan 18, 2025

andreyvelich added this to the v0.18 milestone Jan 20, 2025

github-actions bot removed the lifecycle/stale label Jan 20, 2025

Electronic-Waste mentioned this pull request Jan 21, 2025

[Release] Katib 0.18 Roadmap #2386

Open

7 tasks

shashank-iitbhu force-pushed the feat/optuna-suggestion-service-update branch from edf98f0 to f263b26 Compare January 30, 2025 10:36

shashank-iitbhu marked this pull request as ready for review January 30, 2025 10:40

google-oss-prow bot removed the do-not-merge/work-in-progress label Jan 30, 2025

shashank-iitbhu and others added 4 commits February 3, 2025 18:13

unit test fixed

d4ae140

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

Update pkg/suggestion/v1beta1/hyperopt/base_service.py

99ecbcd

Co-authored-by: Yuki Iwai <yuki.iwai.tz@gmail.com> Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

comment fixed

fd460b6

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

initial logic update

6b18e9b

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

shashank-iitbhu force-pushed the feat/optuna-suggestion-service-update branch from f263b26 to 6b18e9b Compare February 3, 2025 12:46

google-oss-prow bot added size/M and removed size/XL labels Feb 3, 2025

added unit and e2e tests for optuna suggestion service update

2c56864

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

google-oss-prow bot added size/L and removed size/M labels Feb 3, 2025

shashank-iitbhu commented Feb 3, 2025

View reviewed changes

mahdikhashan reviewed Feb 4, 2025

View reviewed changes

refactored code

7bbfc50

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

andreyvelich reviewed Feb 5, 2025

View reviewed changes

shashank-iitbhu added 2 commits February 7, 2025 20:12

added parameter for logUniform and minor changes

49065a5

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

fix

d775c23

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GSOC] `optuna` suggestion service logic update #2446

[GSOC] `optuna` suggestion service logic update #2446

shashank-iitbhu commented Oct 20, 2024

google-oss-prow bot commented Oct 20, 2024

github-actions bot commented Jan 18, 2025

mahdikhashan commented Jan 29, 2025

mahdikhashan commented Jan 30, 2025

mahdikhashan commented Jan 30, 2025

shashank-iitbhu commented Jan 30, 2025

mahdikhashan commented Jan 30, 2025

Electronic-Waste commented Jan 31, 2025

mahdikhashan commented Feb 3, 2025

shashank-iitbhu commented Feb 3, 2025

mahdikhashan commented Feb 3, 2025

andreyvelich commented Feb 3, 2025

shashank-iitbhu Feb 3, 2025

shashank-iitbhu Feb 3, 2025

mahdikhashan Feb 3, 2025

mahdikhashan Feb 4, 2025

mahdikhashan Feb 4, 2025

mahdikhashan Feb 4, 2025 •

edited

Loading

shashank-iitbhu commented Feb 3, 2025 •

edited

Loading

mahdikhashan Feb 4, 2025

mahdikhashan Feb 4, 2025

Electronic-Waste commented Feb 4, 2025

andreyvelich Feb 5, 2025

shashank-iitbhu Feb 7, 2025

andreyvelich Feb 5, 2025

andreyvelich Feb 5, 2025

	elif parameter.type == DOUBLE:
	if parameter.step == "" or parameter.step is None:
	raise Exception(
	"Param {} step is nil; For discrete search space, all parameters "
	"must include step".format(parameter.name)
	)

	elif self.algorithm_name == "grid":
	combinations = HyperParameterSearchSpace.convert_to_combinations(
	self.search_space
	)

	elif param.type in {CATEGORICAL, DISCRETE}:
	elif param.type in [CATEGORICAL, DISCRETE]:

[GSOC] optuna suggestion service logic update #2446

Are you sure you want to change the base?

[GSOC] optuna suggestion service logic update #2446

Conversation

shashank-iitbhu commented Oct 20, 2024

google-oss-prow bot commented Oct 20, 2024

github-actions bot commented Jan 18, 2025

mahdikhashan commented Jan 29, 2025

mahdikhashan commented Jan 30, 2025

mahdikhashan commented Jan 30, 2025

shashank-iitbhu commented Jan 30, 2025

mahdikhashan commented Jan 30, 2025

Electronic-Waste commented Jan 31, 2025

mahdikhashan commented Feb 3, 2025

shashank-iitbhu commented Feb 3, 2025

mahdikhashan commented Feb 3, 2025

andreyvelich commented Feb 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mahdikhashan Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

shashank-iitbhu commented Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Electronic-Waste commented Feb 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

[GSOC] `optuna` suggestion service logic update #2446

[GSOC] `optuna` suggestion service logic update #2446

mahdikhashan Feb 4, 2025 •

edited

Loading

shashank-iitbhu commented Feb 3, 2025 •

edited

Loading