Implement Metadata for SMAC enabling Multi-Fidelity - #771

Why replacing this with a dataframe?

Because configurations are no longer a unique identifier if we support metadata.

An alternative would be to use the type:

Dict[Tuple[ConfigSpace.Configuration, pd.DataFrame], Tuple[TrialInfo, TrialValue]]

but I thought using a dataframe was simpler and more flexible

Not certain you can actually use that Tuple as a Dict key (may not be hashable).

Tuples are hashable, but dataframes are not

Please add some comments regarding the need for the threading lock.

Comments could use some improvement.

Make a new entry of what? What's the first part doing that this one now needs explanation?

These Tuples are getting a little large and hard to read (recall a previous version of this PR where the order of them was mistakenly swapped at one point).

Think we discussed creating a NamedTuple or small DataClass for them instead so that they can be accessed by name in order to make it more readable.

If you want I can do this in this PR, or another follow up PR

I think a predecessor PR would be better. Much like we did with adding the metadata args and named args first.

This is a pretty weird pattern. If we wanted to add more types in the future those kwargs wouldn't work.

What we've done elsewhere is

1. retain the loop over OptimizerType in order to make sure that all optimizers get a test added (or explicitly excluded) whenever we add new optimizer backends.
2. When necessary, within the body of the test set additional parameters in a switch statement for a given optimizer type. e.g.,

if optimizer_type == OptimizerType.SMAC:
  verifier = smac_verify_best
elif optimizer_type == OptimizerType.FLAML:
  pytest.skip("TODO: FLAML Optimizer does not yet support metadata")  # though even here, I think we should test *something*
else:
  raise NotImplementedError(f"Missing test handler for OptimizerType {optimizer_type}")

I have moved the kwargs into the tuple which fixes the kwargs problem.

I find switching over unsupported optimizers messy since it means, in this case, we should skip the entire test as you have above, unless we are testing SMAC. There are other places where we test to make sure that using metadata throws exceptions, so without metadata support there is nothing to test.

But when we add that functionality to another optimizer it will be very easy to forget to enable related testing support, so I'd rather add that now as it will be easier to search for what we need to enable.

Won't it be the same either way? One requires adding a line, the other requires moving a line. I make this more similar to other tests, but I don't think it changes much either way.