support string alg in tune #1093

skzhang1 · 2023-06-27T16:13:33Z

Why are these changes needed?

Support specifying search algorithm by passing the corresponding string in tune.

Related issue number

N/A

Checks

I've included any doc changes needed for https://microsoft.github.io/FLAML/. See https://microsoft.github.io/FLAML/docs/Contribute#documentation to build and test documentation locally.
I've added tests (if relevant) corresponding to the changes introduced in this PR.
I've made sure all auto checks have passed.

sonichi

Please add a test.

sonichi · 2023-06-28T06:25:34Z

flaml/tune/tune.py

- search_alg: An instance of BlendSearch as the search algorithm
- to be used. The same instance can be used for iterative tuning.
+ search_alg: An instance/string of the search algorithm
+ to be used. The same instance/string can be used for iterative tuning.


Suggested change

to be used. The same instance/string can be used for iterative tuning.

to be used. The same instance can be used for iterative tuning.

My understanding is that this sentence is about using the same instance for iterative tuning.

sonichi · 2023-06-28T06:29:02Z

flaml/tune/tune.py

@@ -504,15 +503,19 @@ def easy_objective(config):
 try:
 import optuna as _

- SearchAlgorithm = BlendSearch
+ SearchAlgorithm = BlendSearch if not search_alg else eval(search_alg)


The use of eval is not safe. Could you use an alternative way?

I use assert to make it feasible.

First, I don't like the use of eval. Unless you get an approval from an experience Python engineer, I'm against it. Even locals is better.
Second, the code doesn't look correct in the try...except block. The try part tries to import optuna before using BlendSearch. If the choice is not BlendSearch, we shouldn't enter the try...except block.

sonichi · 2023-06-28T06:34:55Z

flaml/tune/tune.py

+ "BlendSearch",
+ "CFO",
+ ], "If lexico_objectives is not None, search_alg must be CFO or BlendSearch."
+ search_alg.lexico_objectives = lexico_objectives


Shall we implement this in set_search_properties? Otherwise we need to document this way of setting lexico_objectives somewhere. Conceptually I think it should be part of set_search_properties.

Yes. I agree. I support it in CFO for now. Will make it work in BlendSearch in future PR.

qingyun-wu · 2023-06-28T13:58:00Z

flaml/tune/tune.py

@@ -481,12 +481,11 @@ def easy_objective(config):
 else:
 logger.setLevel(logging.CRITICAL)

- from .searcher.blendsearch import BlendSearch, CFO
+ from .searcher.blendsearch import BlendSearch, CFO, RandomSearch


RandomSearch is not used.

we may use it in eval(search_alg)

…ities in CFO

sonichi · 2023-06-29T03:35:07Z

flaml/tune/tune.py

+ "CFO",
+ "CFOCat",
+ "RandomSearch",
+ ], "The string of the search_alg must be feasible."


"feasible" is not clear.

sonichi · 2023-06-29T03:39:16Z

flaml/tune/tune.py

@@ -504,15 +503,19 @@ def easy_objective(config):
 try:
 import optuna as _

- SearchAlgorithm = BlendSearch
+ SearchAlgorithm = BlendSearch if not search_alg else eval(search_alg)


First, I don't like the use of eval. Unless you get an approval from an experience Python engineer, I'm against it. Even locals is better.
Second, the code doesn't look correct in the try...except block. The try part tries to import optuna before using BlendSearch. If the choice is not BlendSearch, we shouldn't enter the try...except block.

sonichi · 2023-06-29T03:41:46Z

flaml/tune/searcher/flow2.py

@@ -317,6 +317,7 @@ def set_search_properties(
 metric: Optional[str] = None,
 mode: Optional[str] = None,
 config: Optional[Dict] = None,
+ lexico_objectives: Optional[Dict] = None,


why is this argument added?

sonichi · 2023-06-29T03:45:32Z

test/tune/test_tune.py

+ # Iterative training function - can be any arbitrary training procedure
+ _evaluation_fn(step, width, height)
+ # Feed the score back back to Tune.
+ print("Trial stopped", step)


test/tune/test_tune.py

sonichi · 2023-06-29T14:17:38Z

flaml/tune/tune.py

+ )
+ metric = lexico_objectives["metrics"][0] or DEFAULT_METRIC
+ else:
+ if not search_alg or search_alg == "BlendSearch":


when search_alg=="BlendSearch", we shouldn't automatically switch to CFO. We should raise an error if the import fails.

yes. resolved.

sonichi · 2023-06-29T14:23:16Z

flaml/tune/tune.py

+ assert search_alg.__class__.__name__ in [
+ "CFO",
+ ], "If lexico_objectives is not None, the search_alg must be CFO for now."


Such assertions and logging happen in different places: here, 519, 487. Could you add TODO notes to modify them later in case they are forgotten?

qingyun-wu · 2023-06-29T14:58:45Z

test/tune/test_tune.py

+ # lexico tune
+ tune.run(
+ _BraninCurrin, search_alg="CFO", num_samples=10, config=mo_search_space, lexico_objectives=lexico_objectives
+ )


Add another test with lexico_objectives andsearch_alg= "BlendSearch"

qingyun-wu · 2023-06-29T15:13:11Z

flaml/tune/tune.py

- logger.warning("If lexico_objectives is not None, search_alg is forced to be CFO")
- search_alg = None
- if search_alg is None:
+ logger.warning("If lexico_objectives is not None, search_alg is forced to be CFO for now.")


Why not put this warning in the place where SearchAlgorithm is forced to be CFO? i.e., immediately after line 518.

support string alg in tune

f45a558

skzhang1 requested review from sonichi and qingyun-wu and removed request for sonichi June 27, 2023 16:13

sonichi reviewed Jun 28, 2023

View reviewed changes

qingyun-wu reviewed Jun 28, 2023

View reviewed changes

add test, enforce string feasible, support lexico in set_search_prior…

acd90ed

…ities in CFO

skzhang1 requested a review from sonichi June 28, 2023 16:12

fix bug

569c532

sonichi reviewed Jun 29, 2023

View reviewed changes

“skzhang1” added 3 commits June 29, 2023 14:35

fix bug

ea8557c

fix bug

1da79e3

fix bug

35f718d

skzhang1 requested a review from sonichi June 29, 2023 08:08

sonichi reviewed Jun 29, 2023

View reviewed changes

qingyun-wu reviewed Jun 29, 2023

View reviewed changes

fix bugs

43729b2

qingyun-wu approved these changes Jun 29, 2023

View reviewed changes

skzhang1 self-assigned this Jun 29, 2023

skzhang1 requested review from feiran-jia and sonichi June 29, 2023 15:51

Merge branch 'main' into string_algorithm

1d2c636

feiran-jia approved these changes Jun 30, 2023

View reviewed changes

sonichi approved these changes Jun 30, 2023

View reviewed changes

sonichi enabled auto-merge June 30, 2023 16:14

sonichi added this pull request to the merge queue Jun 30, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 30, 2023

skzhang1 added this pull request to the merge queue Jun 30, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 30, 2023

skzhang1 added this pull request to the merge queue Jun 30, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 1, 2023

fix yiran

de5637b

skzhang1 enabled auto-merge July 1, 2023 01:01

skzhang1 added this pull request to the merge queue Jul 1, 2023

Merged via the queue into microsoft:main with commit 7a64148 Jul 1, 2023

skzhang1 deleted the string_algorithm branch July 1, 2023 05:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support string alg in tune #1093

support string alg in tune #1093

skzhang1 commented Jun 27, 2023 •

edited by sonichi

Loading

sonichi left a comment

sonichi Jun 28, 2023

skzhang1 Jun 28, 2023

sonichi Jun 28, 2023

skzhang1 Jun 28, 2023 •

edited

Loading

sonichi Jun 29, 2023

sonichi Jun 28, 2023

skzhang1 Jun 28, 2023

qingyun-wu Jun 28, 2023

skzhang1 Jun 28, 2023

sonichi Jun 29, 2023

sonichi Jun 29, 2023

sonichi Jun 29, 2023

skzhang1 Jun 29, 2023

sonichi Jun 29, 2023

skzhang1 Jun 29, 2023

sonichi Jun 29, 2023

skzhang1 Jun 29, 2023

sonichi Jun 29, 2023

skzhang1 Jun 29, 2023

qingyun-wu Jun 29, 2023

skzhang1 Jun 29, 2023

qingyun-wu Jun 29, 2023

skzhang1 Jun 29, 2023

	to be used. The same instance/string can be used for iterative tuning.
	to be used. The same instance can be used for iterative tuning.

support string alg in tune #1093

support string alg in tune #1093

Conversation

skzhang1 commented Jun 27, 2023 • edited by sonichi Loading

Why are these changes needed?

Related issue number

Checks

sonichi left a 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

skzhang1 Jun 28, 2023 • edited Loading

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

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

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

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

skzhang1 commented Jun 27, 2023 •

edited by sonichi

Loading

skzhang1 Jun 28, 2023 •

edited

Loading