Skip to content

Simplify flow operators internal design #153

@benbovy

Description

@benbovy

(following discussions with @adriendelsalle)

While flow operators are both flexible and user-friendly, the internal design (interface vs. implementation classes, type erasure, etc.) is very convoluted with a lot of indirection.

It also makes harder the implementation of flow operators in 3rd-party code (C++ or Python). Not sure it is a good idea to allow this, though, as implementing a flow operator requires read/write access to the internal data structures of the flow graph implementation, which is risky to expose publicly (at least let's keep it clear that it is only for adventurous users and that it may break at any time).

Nevertheless, It would be great to see the flow operator internal logic streamlined so that it becomes less an obstacle for new contributors to get onboard. However, every time I think about it I hit some painful blockers.

A nice alternative would be to have a single flow_operator base class (i.e., no more distinct interface and implementation classes) with dynamic polymorphism. However, we would need to figure out how to pass the flow graph (implementation) object to .apply() and .save() and work around the impossibility to add virtual template member functions in C++, e.g.,

  • further refactor flow_graph and flow_graph_impl in order to remove some template parameters, although I'm not sure that there is an easy way to fully eliminate the grid type parameter G.
  • turn flow operator into a template class flow_operator<FG> (where FG is the flow graph implementation type), although I like the current way of creating a flow operator object independently of any graph (grid) type or object.
  • any other idea @adriendelsalle?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions