Deprecate creation of empty Wall or FlowDevice objects in C++

[ischoegl]

Abstract

Creation of empty Wall, FlowDevice and ReactorSurface objects is not supported by the Python API, and Cantera/cantera#1765 removes preliminary support from experimental MATLAB. It would be consistent to disallow this at the C++ level as well, which would (eventually) simplify/streamline the interface by removing several methods and associated exception handling. Passing shared pointers to ReactorBase objects to constructors instead would be a significant step towards elimination of raw pointers from the reactor code base. A similar argument holds for ReactorNet and the pending ReactorEdge (see Cantera/cantera#1697).

Motivation

Describe the need for the proposed change:

• What problem is it trying to solve? ... simplify reactor API
• Who is affected by the change? ... mostly developers
• Why is this a good solution? ... simplifies API

Possible Solutions

Update constructors and factory methods and clib beyond changes introduced in Cantera/cantera#1765. Deprecate various install methods. Constructors should be updated to use signatures similar to

FlowDevice(shared_ptr<ReactorBase> upstream=nullptr, shared_ptr<ReactorBase> downstream=nullptr, 
           const string& name="(none)")

where null upstream/downstream will raise deprecation warnings in 3.1, with defaults being removed thereafter.

References

• Reproducible names for zeroD objects cantera#1765
• Implementation of 'ReactorEdge' class cantera#1697

[ischoegl]

@speth … could you have a look at Cantera/cantera#1765, so I can move ahead and tackle this (as well as deprecate a bunch of raw pointered interfaces) prior to the release of 3.1?

[speth]

(repeating some comments from my review of Cantera/cantera#1788 here, since I think this is a better place for discussion)

I like the idea of always providing the Reactor objects to the constructor for the connector objects and bringing that feature of the Python API to all the interfaces. However, it's not quite clear to me what the benefit is of making all Reactor and connector objects into mandatory shared_ptrs.

Currently, the ownership model is simple: none of the Reactor/ReactorNet/FlowDevice/etc. objects take ownership of anything they are connected to; the lifetimes of all these objects are managed by the caller. Within this set of objects, we need connections in all directions. Avoiding reference cycles will require the use of weak_ptr for many of the connections, with the associated overhead of "upgrading" those to shared_ptr where they are used. And you will still be left with the situation where it's possible to hold on to some element of the network but not be able to use it because the rest of the network has been deleted, though at least that would be checked and generate an exception rather than undefined behavior.

[ischoegl]

Thanks for the review of Cantera/cantera#1788 - before I address change requests, I wanted to briefly follow up on the shared_ptr issue. At the moment, whatever is 'under the hood' of various 0D objects in the current implementation is a mix of references, raw pointers, etc., which I was hoping to replace with a more consistent approach once the interfaces allow for it. It is true that there is the issue of cyclical references that need to be avoided. At the same time, I am not sure what the alternative would be if we want to stop using raw pointers - giving ReactorNet shared ownership of an object would imho create a clear structure, which would be safer than relying on the existence of externally managed objects. I personally see the external API only as a way to instantiate objects, with the C++ core having the requirement to always handle things safely.

Regarding the implementation under the hood, I think we need to increase the importance of ReactorNet in handling interactions, so we can avoid the doubly-linked list issues you point out. From that perspective, Connectors need to know what's in their adjacent Reactor nodes (i.e. a shared_ptr would be nice to have), but Reactor objects only need to know fluxes when integrating their governing equations. The calculation could be done in two steps: (1) evaluate connectors and build a global source term, and (2) evaluate governing equations. While this would a shift in paradigms, I believe it would help with #202. This is admittedly not a completely thought-through approach, but I ultimately believe that we need to get away from raw pointers and references to future-proof the code base.

[ischoegl]

For further thoughts on anticipated implementations see #201 (comment)