How do we couple benchmark functions and their spaces?

[SRFU-NN]

As it is now, the benchmark functions in ./ProcessOptimizer/model_systems/benchmarks.py have their bounds defined in their docstrings. Is this what we want?

[SRFU-NN]

We could define ModelSystems from them, so you can just grab the entire ModelSystem.

This could either be done in the same module (advantage: reading comprehension, the space is defined close to the scoring function), or in a separate module (advantage: more coherent modules, simpler module if you just want the scoring functions).

[dk-teknologisk-mon]

I'm starting to think that we may want to build the space, min and max values into the scoring functions. Just to give an example of what I have to do now in order to build several model systems in preparation for benchmarking:

# Prepare settings of the peaks model system
peaks_space = [(-3.0, 3.0), (-3.0, 3.0)]
peaks_min = -6.5511
peaks_max = 8.106
peaks_noise = {
    "model_type": "constant",
    "noise_size": 0.2,
    }
# Build ModelSystem object
peaks_model = ModelSystem(
    score=peaks, 
    space=peaks_space, 
    true_min=peaks_min, 
    true_max=peaks_max,
    noise_model=peaks_noise,
)

# Prepare poly2 system
poly2_space = [(-1.0, 1.0), (-1.0, 1.0)]
poly2_min = -2.051
poly2_max = -1.27
poly2_noise = {
    "model_type": "constant",
    "noise_size": 0.2,
    }
# Build ModelSystem object
poly2_model = ModelSystem(
    score=poly2,
    space=poly2_space,
    true_min=poly2_min,
    true_max=poly2_max,
    noise_model=poly2_noise,
)

# Prepare Branin Hoo system
branin_space = [(-5.0, 10.0), (0.0, 15.0)]
branin_min = 0.3979
branin_max = -1.27
branin_noise = {
    "model_type": "constant",
    "noise_size": 0.2,
    }
# Build ModelSystem object
branin_model = ModelSystem(
    score=branin,
    space=branin_space,
    true_min=branin_min,
    true_max=branin_max,
    noise_model=branin_noise,
)

# Gather model systems
model_system_list = [
    peaks_model,
    poly2_model,
    branin_model,
]

Having to do this manually gets old really fast, so I think it would make sense to bulk up the benchmark functions to a benchmark class instead, so building the ModelSystem gets down to something like:

branin_noise = {
    "model_type": "constant",
    "noise_size": 0.2,
}
# Build ModelSystem object
branin_model = ModelSystem(
    benchmark=branin,
    noise_model=branin_noise,
)

I guess the practical implementation and interaction with ModelSystem would have the latter check whether a "benchmark" object was passed to it, and if not it should require you to pass a scoring function and a space instead.

[SRFU-NN]

We could also define the relevant ModelSystems directly in the benchmark.py file. Then it just has to be done once.

That would also mean that we define what noise is probably a good starting point when we define the benchmark, which also seems like a good idea - You probably have a better feeling for that when you define the benchmark than when you use it.

And if we do that, we should probably make a method to easily change the noise function of a benchmark, so I can just grab the one I need, and then change it to e.g. proportional noise.

[dk-teknologisk-mon]

I like your suggestions. On paper it appears very easy to add a method that sets/updates the noise model in the model system, and then just let each benchmark be returned as a fully prepared model system when it is called.

This change imposes a fair bit for changes in the test files, but should otherwise be realtively easy to get done.

[dk-teknologisk-mon]

The main question I want to make sure we agree on is that the score functions will be built into the construction of their respective model system and as a result will stop existing as functions you can import elsewhere in the package. If we think it is important that they are separate then I would suggest we change the name of benchmarks.py to something like benchmark_functions.py and use a benchmark_models.py for constructing our default takes on the Branin, Hartmann etc. ModelSystem objects.

[SRFU-NN]

Thinking more about it, shouldn't we import all of them relevant model systems into __init__.py in model_systems? So you can do

from ProcessOptimizer.model_systems import Branin

to get the default Branin-Hoo model system.

That also makes for a convenient way to handle the existing score functions: Simply keep the existing benchmarks.py, but mark them as deprecated.

If we are to be VERY strictly pythonic (which we shouldn't necessarily), the model systems could them be in separate files, with score, noise, and everything. The model systems can then be imported into __init__.py. Then we have a nice, clean interface, and nice, simply files, and each file being responsible for one thing.

This is a bit of a bigger change, so it might not be worth it. And if we do it all now, we will have duplicate code, unless we are tricky about benchmarks.py. I think the duplicate code is OK, since one copy is not being maintained.

[SRFU-NN]

And this leads to a very backwards compatible way to do it:

Convert the existing score functions to model systems named X_model_system in benchmarks.py, keeping the score functions.

Import the new model systems into __init__.py with

from benchmarks import X_model_system as X

For new model systems, define them in individual files, and import them into __init__.py

The only downside is that legacy benchmarks are in benchmarks.py, while new benchmarks are in separate files.

[SRFU-NN]

Having score functions and models in separate files would make the individual files more uniform, but it will also make them harder to read, since you have to go to two files to get both the score function and the space. So I prefer having them in the same file.

I would suggest some standard naming instead. You used X and X_model in your example, which I think is a good way to do it. If I were starting over, I would go with X_score for the score function and X for the model, since we want people to use the model unless they really know what they are doing, but for backward compatibility, the other naming scheme is acceptable.

I am not that worried about use in other parts of the package, the IDE should be able to find all of those. But we aren't sure how they are used elsewhere. but maybe I am too cautious?

[SRFU-NN]

Completed in #189

We have chosen to break backward compatibility when importing benchmark systems.