Draft: Unify Graph class hierarchy
Created by: gkirgizov
That's an important PR that starts big changes in the core of the core: Graph hierarchy design.
My analysis of the current situation with Graphs: There is a lot of explicit functionality that tries to achieve the following points:
- Draw a clear boundary between Optimiser & problem domain (AutoML/EPDE in future/etc) -- done through duplicated Graph & OptGraph classes (same for GraphNode & OptNode classes)
- Share single Graph implementation -- done through GraphOperator & NodeOperator classes. Graph/OptGraph just delegated these calls to Operators.
Yet this approach is not without its problems:
- Graph & OptGraph still duplicate code, that complicates maintenance and further development of the framework core.
- GraphOperator requires cyclic reference to the owned graph, that complicates some code (e.g. serialization).
- Adapter functionality becomes more complex without good reason -- in effect, it just implements runtime transformations between classes where they could be avoided altogether (i.e. Pipeline is already a Graph, which is the same as OptGraph)
Same points can be achieved with a cleaner design: What previously was done manually (sharing of graph implementation between OptGraph & Graph through the GraphOperator) could be actually encoded in class hierarchy. So now instead of runtime transformations and delegation to GraphOperator everything is encoded in class hierarchy, while implementation details are hidden behind opaque Graph interface.
Additional benefits:
- We can have more (and maybe external) Graph implementations. For example, now it's in principle possible to add network-x Graph implementation, which would give us more guarantees in correctness and efficiency of graph operations.
The continuation of this work will be #742 . Place of Adapters will be preserved and we still will be able to optimise completely foreign structures -- as far as they can be translated to Graph. But now framework users will have additional choice: if their optimised structure conforms to the Graph interface, no adaptation is required, as Optimiser will work through the Graph interface.
Specific notes on my proposed changes:
- Graph interface is introduced
- GraphOperator becomes the only implementation of Graph
- GraphDelegate provides inheritance through composition -- in effect, substitutes what was done previously through delegation to
self.operator
- OptGraph now is just a wrapper that adds
self.log
to Graph -- could remove it after #622 (closed), and just make it an alias of GraphDelegate - Introduce
Copyable
mixin-class. GraphDelegate & GraphOperator inherit from it. (that's instead of direct impl of copy & deepcopy in OptGraph) - OptNode is just an alias for GraphNode
- NodeOperator is dropped (now it's unnecessary because there's only one GraphNode impl)
- Avoid the need for explicit passing of
DEFAULT_PARAMS_STUB
in OptNode and slightly refactor handling of it
Fixes #741