-
-
Notifications
You must be signed in to change notification settings - Fork 349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Link Python module to Cantera shared library #1429
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
speth
force-pushed
the
python-link-shared
branch
from
January 24, 2023 15:29
8201853
to
1f1d26f
Compare
Codecov Report
@@ Coverage Diff @@
## main #1429 +/- ##
==========================================
+ Coverage 70.73% 70.94% +0.21%
==========================================
Files 378 369 -9
Lines 55141 55201 +60
Branches 18164 18176 +12
==========================================
+ Hits 39006 39165 +159
+ Misses 13626 13574 -52
+ Partials 2509 2462 -47
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
speth
force-pushed
the
python-link-shared
branch
7 times, most recently
from
January 24, 2023 21:44
f6080c6
to
1fc2b7a
Compare
5 tasks
speth
force-pushed
the
python-link-shared
branch
18 times, most recently
from
February 1, 2023 04:56
8485cb3
to
508967a
Compare
This is a prerequisite to using Boost.DLL with the std::filesystem library.
This eliminates the need to directly link the Cantera shared library or Cantera applications to libpython.
This version properly exports all necessary DLL symbols, eliminating the need to embed it separately in the Cython module.
10.15 or newer is required for C++17 support
This requires copying the relevant MinGW runtime libraries into the Python module.
Trying this to understand why this step can sometimes take upwards of 8 minutes.
This is important because extensions can only be loaded correctly when Cantera is linked as a shared library.
When the Python module is linked to the Cantera shared library, it is possible that a user has multiple incompatible versions of the library installed. This checks that the Cantera version and Git commit are the same when importing the Python module, to avoid crashes or erroneous behavior due to ABI mismatches.
speth
force-pushed
the
python-link-shared
branch
from
February 2, 2023 22:54
9c05ade
to
3dd702d
Compare
ischoegl
approved these changes
Feb 2, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes proposed in this pull request
The main purpose of this PR is to modify the implementation of
ExtensibleRate
so that the core Cantera library (libcantera
) does not need to depend on a specific Python installation at compilation / installation time. Achieving this required some fairly far-reaching changes, though I think they will all have long-term benefits.PythonExtensionManager
class and associated Cython functions have been moved out oflibcantera
into a separate shim,libcantera_python
that is loaded on demand. This new library is the one that links tolibpython
and loads the Cantera Python module.libcantera_python
) both depend onlibcantera
,libcantera
needs to be loaded as a shared library to avoid having two copies of the library with separate state (which would interfere instantiating user defined types that are only instance with one copy ofReactionRateFactory
, for example). Multiple versions of Python are supported (not at the same time!) by having multiple versions oflibcantera_python
, e.g.libcantera_python3_11.so
andlibcantera_python3_10.so
.libcantera_shared
requires "exporting" the complete C++ interface. Due the the sheer number of methods that need to be exported, rather than the standard approach investigated in Export C++ library symbols for core methods #1298, this PR uses the static library to generate a list of symbols to be exported from the DLL.libcantera_python
library on-demand are OS-specific. Rather than use these low-level functions, the Boost.DLL library is used for its higher-level, cross-platform capabilities. While Boost.DLL is header-only, by default it depends on Boost.Filesystem, which has a compiled component. However, since C++17 introducedstd::filesystem
(derived from Boost.Filesystem), Boost.DLL provides an option to usestd::filesystem
. Therefore, in order to avoid introducing a dependence on the compiled components of Boost, this PR bumps Cantera's compiler requirement to the C++17 standard.There are also a few more minor changes that ended up coming along for the ride:
using
directives within theCantera
namespace for some of the most-used standard library types:string
,vector
,map
,function
,pair
,unique_ptr
. I figuremap<string, string>
is more readable thanstd::map<std::string, std::string>
.fmt
andEigen
.If applicable, fill in the issue number this pull request is fixing
Checklist
scons build
&scons test
) and unit tests address code coverage