Fix mixed numeric datatypes for optimizers

[ephoris]

Addressing issue discussed in #666

[ephoris]

@microsoft-github-policy-service agree

[motus]

Looks nice! Next step, we should make sure LlamaTune does not choke on ConfigSpace instances that have conditionals (like, with the tunables that have special values). For starters, LlamaTune should be able to support that input: tunable_to_configspace_test.py:32

[ephoris]

Hm okay I have noticed that the llamatune_opt_test.py:49 failed. Looking into it, I see that some test in mlos_core_opt_df_test.py:45 define parameters with special characters. For example kernel_sched_migration_cost_ns!type. Normally this column name is fine if we're converting from pandas.DataFrame to dict converting, but using a NamedTuples received from itertuples() will parse these columns strings with undefined behavior, which is why I am getting a test failure. I think we might have to use a different solution instead of pandas.DataFrame.itertuples().

[ephoris]

Since I am assuming MLOS wants to support generic strings for hyper parameters, we can revert back to iterrows(). Then by typecasting the dataframe to the object type, we can preserve numeric dtypes when calling iterrows() (see smac_optimizers.py:339). It's not as elegant unfortunately, but this prevents pandas from upcasting integers to floats.

[ephoris]

Linter failed on 983763f. Fixed then reran linter locally, sorry for the spam. I think workflows should pass this time around.

[bpkroth]

Hm okay I have noticed that the llamatune_opt_test.py:49 failed. Looking into it, I see that some test in mlos_core_opt_df_test.py:45 define parameters with special characters. For example kernel_sched_migration_cost_ns!type. Normally this column name is fine if we're converting from pandas.DataFrame to dict converting, but using a NamedTuples received from itertuples() will parse these columns strings with undefined behavior, which is why I am getting a test failure. I think we might have to use a different solution instead of pandas.DataFrame.itertuples().

You could also try using itertuples(name=None) to get non-NamedTuples back.

[bpkroth]

Linter failed on 983763f. Fixed then reran linter locally, sorry for the spam. I think workflows should pass this time around.

No worries. I generally suggest using the devcontainer and just locally running make check and make test when you're doing things. It should run the same tests we did in CI then.

[bpkroth]

Final thought: I think there's a few places in mlos_bench where we do a to_numeric that might need re-examine with this.

[bpkroth]

LGTM. Thanks!

[bpkroth]

Final thought: I think there's a few places in mlos_bench where we do a to_numeric that might need re-examine with this.

We can take this up elsewhere I think.

[ephoris]

Hm okay I have noticed that the llamatune_opt_test.py:49 failed. Looking into it, I see that some test in mlos_core_opt_df_test.py:45 define parameters with special characters. For example kernel_sched_migration_cost_ns!type. Normally this column name is fine if we're converting from pandas.DataFrame to dict converting, but using a NamedTuples received from itertuples() will parse these columns strings with undefined behavior, which is why I am getting a test failure. I think we might have to use a different solution instead of pandas.DataFrame.itertuples().

You could also try using itertuples(name=None) to get non-NamedTuples back.

Unfortunately, itertuples preserves dtypes, but it also preserves pandas.NA values. Normally I would say this is good, however, pandas.NA value is not treated the same as None values. This results in situations where the ConfigSpace package will throw errors as it only handles None values. For now I guess pandas.DataFrame.astype('O') might be the most elegant solution for the time being as iterrows() will implicitly convert pandas.NA to None values which most of the code paths were set up to deal with.

[ephoris]

Thanks @bpkroth and @motus. Great learning experience on my end, I hope this bug fix will be helpful and prevent errors in the future.