Skip to content

Fix 'list index out of range' Error with 'mlflow_logging': True and 'max_iter': 1 #1417

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.

Sign up for GitHub

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

Closed
Stickic-cyber wants to merge 3 commits into from
Closed

Fix 'list index out of range' Error with 'mlflow_logging': True and 'max_iter': 1 #1417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1374b68
Fix MLflow best run ID issue
Stickic-cyber Apr 2, 2025
aab3aee
Fix mlflow best_run_id issue
Stickic-cyber Apr 3, 2025
27059c1
Fix issue with mlflow_best_run_id
Stickic-cyber Apr 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions flaml/automl/automl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2521,8 +2521,8 @@ def _search(self):
self._warn_threshold = 10
self._selected = None
self.modelcount = 0
if self._max_iter < 2 and self.estimator_list and self._state.retrain_final:
# when max_iter is 1, no need to search
if self._max_iter < 1 and self.estimator_list and self._state.retrain_final:
# when max_iter is 1, search is also necessary for test. Besides, it won't take a lot of time.
self.modelcount = self._max_iter
self._max_iter = 0
self._best_estimator = estimator = self.estimator_list[0]
Expand Down
14 changes: 10 additions & 4 deletions flaml/fabric/mlflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,16 +504,22 @@ def log_automl(self, automl):
self.adopt_children(automl)

if self.manual_log:
best_mlflow_run_id = self.manual_run_ids[automl._best_iteration]
if len(self.manual_run_ids) == 0:
best_mlflow_run_id = self.parent_run_id or mlflow.active_run().info.run_id
Copy link
Copy Markdown
Collaborator

@thinkall thinkall Apr 3, 2025

Choose a reason for hiding this comment

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

In current design, max_iter = 1 makes no sense to FLAML. There will be no mlflow run created for it. When max_iter>1, self.mlflow_integration.record_state(self, search_state, estimator) will be called to create mlflow runs to record states.

The key here is that no run is created and appended to manual_run_ids. Set it to parent_run or current active run is not correct.

I see two ways to fix the issue:

  1. Don't skip search when max_iter = 1, it's acceptable as one trial won't take a lot of time, thus won't affect the test process (I think people set max_iter to 1 for quick test purpose).

  2. Add mlflow_integration.record_state to the logic of max_iter < 2.

Need to test which one is better, i.e., won't introduce new bugs.

Copy link
Copy Markdown
Collaborator

@thinkall thinkall Apr 4, 2025

Choose a reason for hiding this comment

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

@Stickic-cyber it seems that you missed my detailed comments on your code changes.

else:
best_mlflow_run_id = self.manual_run_ids[automl._best_iteration]
best_run_name = self.mlflow_client.get_run(best_mlflow_run_id).info.run_name
automl.best_run_id = best_mlflow_run_id
automl.best_run_name = best_run_name
self.mlflow_client.set_tag(best_mlflow_run_id, "flaml.best_run", True)
self.best_run_id = best_mlflow_run_id
if self.parent_run_id is not None:
conf = automl._config_history[automl._best_iteration][1].copy()
if "ml" in conf.keys():
conf = conf["ml"]
conf = {}
if automl._best_iteration in automl._config_history:###
Copy link
Copy Markdown
Collaborator

@thinkall thinkall Apr 3, 2025

Choose a reason for hiding this comment

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

If we do search for max_iter=1, this change won't be needed.

conf = automl._config_history[automl._best_iteration][1].copy()
if "ml" in conf.keys():
conf = conf["ml"]


mlflow.log_params(conf)
mlflow.log_param("best_learner", automl._best_estimator)
Expand Down