@speth ... the interface looks quite promising!

Regarding further extensions: in CustomRate (which is de-facto superseded by this PR, and for good reason), one of the to-do items I have been mulling over (but never realized) was how to cleanly incorporate other properties, e.g. pressure or concentrations. It turns out that for CustomRate you can actually retrieve things from Solution while calculating rates with lambda functions, even if it is somewhat messy. This is not an option here ... what is your general thinking for an interface that goes beyond T? (Obviously this should not be addressed in this PR).

Regarding further extensions: in CustomRate (which is de-facto superseded by this PR, and for good reason), one of the to-do items I have been mulling over (but never realized) was how to cleanly incorporate other properties, e.g. pressure or concentrations. It turns out that for CustomRate you can actually retrieve things from Solution while calculating rates with lambda functions, even if it is somewhat messy. This is not an option here ... what is your general thinking for an interface that goes beyond T?

My plan is to provide an interface much like we use in C++: Add a Python ExtensibleRateData class that has an overloadable update method that receives a Solution object, which can extract any store any data needed for rate evaluation (temperature, pressure, species enthalpies, etc), which gets called once during a full set of rate evaluations. This object then gets passed as the argument to ExtensibleRate.replace_eval instead of just the temperature.

At least so far, the biggest complication I see is the need to wrap existing C++ ThermoPhase and Kinetics objects as a Solution object that is owned from the C++ side, but I think it's manageable, and I've already got a simpler version of that implemented here, since the ReactionRateDelegator object has to manage the lifetime of the Python ExtensibleRate object.

Codecov Report

Merging #1382 (d7efc72) into main (c91e9eb) will decrease coverage by 0.18%.
The diff coverage is 84.96%.

@@            Coverage Diff             @@
##             main    #1382      +/-   ##
==========================================
- Coverage   71.04%   70.85%   -0.19%     
==========================================
  Files         359      363       +4     
  Lines       51707    51837     +130     
  Branches    17327    17354      +27     
==========================================
- Hits        36735    36729       -6     
- Misses      12634    12716      +82     
- Partials     2338     2392      +54     

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

As is evident from the C++ test, the availability of the Python extension is not dependent on the installation of Cantera's Python API.

This is not quite correct. To implement a Python extension, the user's class must derive from ct.ExtensibleRate, or, as this grows to cover other classes, a Cython class that holds an object derived from the C++ Delegator class. Furthermore, as I mentioned in the plans for ExtensibleRate, the plan is that the ExtensibleRateData class will receive a (Python) Solution object as input, which of course implies access to core pieces of the Cantera Python interface.

To implement a Python extension, the user's class must derive from ct.ExtensibleRate, or, as this grows to cover other classes, a Cython class that holds an object derived from the C++ Delegator class.

True (sadly).

Looks like the changes broke conda-recipes, see Action output ... 😢

undefined symbol: _ZN7Cantera22PythonExtensionManager25registerPythonRateBuilderERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES8_S8_

I don't understand why this should be a problem, given that the Cantera/cantera CI is passing. The referenced function is defined on Line 172 of PythonExtensionManager.cpp. The only "interesting" differences that I can think of are:

• this is a static member function, unlike most functions defined in the library.
• For the conda recipes, we're dynamically linking the Python module against the main library, rather than linking to the static library.

I don't think this should inherently be a problem, but apparently something isn't right here.

🤷‍♂️ … I don’t know why this is failing either. Some of the other build scripts that are run upon a merge (MSI, PyPI, macOS) are failing also, but I honestly have never looked at those before, and there may be other things at play.

Oh, I see what the problem is. In the conda-recipes build, the shared library is built with the Python module disabled, but that is currently what also determines whether support for Python-based extensions is compiled. I thought that made sense, since the requirements for Python extension support are the same as those for the Python module itself, and using the Python extension support requires the Python module to be present.

The other thing that has changed that isn't accounted for yet in the conda recipe is that libcantera now depends on a specific version of libpython, so we can't provide just one version of libcantera for all Python versions.

🎉 … makes sense

the shared library is built with the Python module disabled, but that is currently what also determines whether support for Python-based extensions is compiled.

When I made the (incorrect) comment about the Python API not being required for extensions to work, I was coming from a similar direction. So libcantera should receive Python support … does it even make sense to create the package as a separate entity, especially given that libcantera-devel depends on it also?

This probably should go into an issue report on conda-recipes at this point.