DEV: Suggested Edits for #343

[flongford]

Description

Related Issues

342 #343

Background

Suggested usage of IOptimizer classes - although #342 originally planned to have a mixin based system, it may make better sense to use Interface classes instead.

Summary

Rather than using these classes as mixins for BaseOptimizerEngine subclasses, we include them as traits.

This also allows for reallocation of a library backend during runtime.

Notes:

I am sure there are some confusing naming conventions going on here, happy to hear any alternatives

Future Work

• Update existing PRs for Nevergrad and ITWM MCO engines
• Expose IOptimizer classes in the UI

Changes

• Moves initial_parameter_value and parameter_bounds to BaseOptimizerEngine
• Includes optimizer attribute on WeightedOptimizerEngine and APosterioriEngine classes, which must fulfil IOptimizer interface
• Removes optimize_function method from all BaseOptimizerEngine subclasses, defers usage to self.optimizer.optimize_function instead

[codecov[bot]]

Codecov Report

Merging #344 into optimizer-mixin will decrease coverage by 0.00%. The diff coverage is 93.75%.

@@                 Coverage Diff                 @@
##           optimizer-mixin     #344      +/-   ##
===================================================
- Coverage            98.09%   98.08%   -0.01%     
===================================================
  Files                   63       63              
  Lines                 1733     1727       -6     
  Branches               176      174       -2     
===================================================
- Hits                  1700     1694       -6     
  Misses                  29       29              
  Partials                 4        4              

Impacted Files	Coverage Δ
.../optimizer_engines/aposteriori_optimizer_engine.py	62.50% <75.00%> (-10.23%)	:arrow_down:
...dss/mco/optimizer_engines/base_optimizer_engine.py	100.00% <100.00%> (ø)
...mco/optimizer_engines/weighted_optimizer_engine.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 972615c...457b890. Read the comment docs.

[flongford]

Note: this is failing on coverage only, I am going to merge and then we can work with this in #343

[sparsonslab]

Bear in mind, that by making the optimizer an instance of the engine, rather than a mixin, it will change the way the optimizer's attributes (algorithms, etc) are exposed to any MCO or MCOModel. With the mixin, those attributes are inherited by the engine and so just appear like they used to, e.g. engine.algorithms. Now you have engine.optimizer.algorithms.

[flongford]

Bear in mind, that by making the optimizer an instance of the engine, rather than a mixin, it will change the way the optimizer's attributes (algorithms, etc) are exposed to any MCO or MCOModel. With the mixin, those attributes are inherited by the engine and so just appear like they used to, e.g. engine.algorithms. Now you have engine.optimizer.algorithms.

This is true, we will need to address this when deciding on how to fit the BaseOptimizerEngine and IOptimizer classes into the BDSS UI framework.

[sparsonslab]

I've also realised we don't need to pass x0 and bounds as args to optimize_function. All we need to pass is self.parameters, as these contain both x0 and bounds.

[flongford]

I've also realised we don't need to pass x0 and bounds as args to optimize_function. All we need to pass is self.parameters, as these contain both x0 and bounds.

True, though it depends whether we want the IOptimizer to be exposed as a public interface. If it's completely public, then it shouldn't require anything inside of the force_bdss package to run.