Skip to content

Commit

Permalink
Fix storage API failing to return DF for experiments without any tuna…
Browse files Browse the repository at this point in the history
…bles (#889)

# Pull Request

## Fix storage API failing to return DF for experiments without any
tunables

Addresses #884 

---

## Description

When an experiment is run without any tunables (benchmarking with
default parameters), the storage API fails to return a dataframe of the
results.

Example error:
```
>>> exp = storage.experiments["eujingchua-bench-57-02"]
>>> exp.results_df
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/workspaces/MySQL-Autotuning/MLOS/mlos_bench/mlos_bench/storage/sql/experiment_data.py", line 212, in results_df
    return common.get_results_df(self._engine, self._schema, self._experiment_id)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/MySQL-Autotuning/MLOS/mlos_bench/mlos_bench/storage/sql/common.py", line 183, in get_results_df
    ExperimentData.CONFIG_COLUMN_PREFIX + row.param_id,
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
TypeError: can only concatenate str (not "NoneType") to str
```

Going to the relevant line and inserting a breakpoint, the relevant
columns are `["trial_id", "tunable_config_id", "param", "value"]`.
We can see the data is full of NULLs / Nones
```
(Pdb) results = configs.fetchall()
(Pdb) results
[(1, 6541, None, None), (2, 6541, None, None), (3, 6541, None, None), (4, 6541, None, None), (5, 6541, None, None), (6, 6541, None, None), (7, 6541, None, None), (8, 6541, None, None), (9, 6541, None, None), (10, 6541, None, None), (11, 6541, None, None), (12, 6541, None, None), (13, 6541, None, None), (14, 6541, None, None), (15, 6541, None, None), (16, 6541, None, None), (17, 6541, None, None), (18, 6541, None, None), (19, 6541, None, None), (20, 6541, None, None), (21, 6541, None, None), (22, 6541, None, None), (23, 6541, None, None), (24, 6541, None, None), (25, 6541, None, None), (26, 6541, None, None), (27, 6541, None, None), (28, 6541, None, None), (29, 6541, None, None), (30, 6541, None, None), (31, 6541, None, None), (32, 6541, None, None), (33, 6541, None, None), (34, 6541, None, None), (35, 6541, None, None), (36, 6541, None, None), (37, 6541, None, None), (38, 6541, None, None), (39, 6541, None, None), (40, 6541, None, None), (41, 6541, None, None), (42, 6541, None, None), (43, 6541, None, None), (44, 6541, None, None), (45, 6541, None, None), (46, 6541, None, None), (47, 6541, None, None), (48, 6541, None, None), (49, 6541, None, None), (50, 6541, None, None), (51, 6541, None, None), (52, 6541, None, None), (53, 6541, None, None), (54, 6541, None, None), (55, 6541, None, None), (56, 6541, None, None), (57, 6541, None, None), (58, 6541, None, None), (59, 6541, None, None), (60, 6541, None, None), (61, 6541, None, None), (62, 6541, None, None), (63, 6541, None, None), (64, 6541, None, None), (65, 6541, None, None), (66, 6541, None, None), (67, 6541, None, None), (68, 6541, None, None), (69, 6541, None, None), (70, 6541, None, None), (71, 6541, None, None), (72, 6541, None, None), (73, 6541, None, None), (74, 6541, None, None), (75, 6541, None, None), (76, 6541, None, None), (77, 6541, None, None), (78, 6541, None, None), (79, 6541, None, None), (80, 6541, None, None), (81, 6541, None, None), (82, 6541, None, None), (83, 6541, None, None), (84, 6541, None, None), (85, 6541, None, None), (86, 6541, None, None), (87, 6541, None, None), (88, 6541, None, None), (89, 6541, None, None), (90, 6541, None, None), (91, 6541, None, None), (92, 6541, None, None), (93, 6541, None, None), (94, 6541, None, None), (95, 6541, None, None), (96, 6541, None, None), (97, 6541, None, None), (98, 6541, None, None), (99, 6541, None, None), (100, 6541, None, None)]
```



---

## Type of Change

_Indicate the type of change by choosing one (or more) of the
following:_

- 🛠️ Bug fix

---

## Testing

Unit test added covering this case, and also manual testing.

---

## Additional Notes (optional)

_Add any additional context or information for reviewers._

---

---------

Co-authored-by: Eu Jing Chua <[email protected]>
Co-authored-by: Sergiy Matusevych <[email protected]>
  • Loading branch information
3 people authored Dec 5, 2024
1 parent b66e134 commit c9fa067
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
1 change: 0 additions & 1 deletion mlos_bench/mlos_bench/storage/sql/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ def get_results_df(
.join(
schema.config_param,
schema.config_param.c.config_id == schema.trial.c.config_id,
isouter=True,
)
.order_by(
schema.trial.c.trial_id,
Expand Down
13 changes: 13 additions & 0 deletions mlos_bench/mlos_bench/tests/storage/exp_data_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,19 @@ def test_exp_data_results_df(exp_data: ExperimentData, tunable_groups: TunableGr
)


def test_exp_no_tunables_data_results_df(exp_no_tunables_data: ExperimentData) -> None:
"""Tests the results_df property of ExperimentData when there are no tunables."""
results_df = exp_no_tunables_data.results_df
expected_trials_count = CONFIG_COUNT * CONFIG_TRIAL_REPEAT_COUNT
assert len(results_df) == expected_trials_count
assert len(results_df["trial_id"].unique()) == expected_trials_count
obj_target = next(iter(exp_no_tunables_data.objectives))
assert (
len(results_df[ExperimentData.RESULT_COLUMN_PREFIX + obj_target]) == expected_trials_count
)
assert not results_df.columns.str.startswith(ExperimentData.CONFIG_COLUMN_PREFIX).any()


def test_exp_data_tunable_config_trial_group_id_in_results_df(exp_data: ExperimentData) -> None:
"""
Tests the tunable_config_trial_group_id property of ExperimentData.results_df.
Expand Down

0 comments on commit c9fa067

Please sign in to comment.