Gas transport polynomial fits in Python #1077
Conversation
…operties polynomial fits. Also made those functions accessible from the python interface.
Codecov Report
@@ Coverage Diff @@
## main #1077 +/- ##
==========================================
+ Coverage 73.07% 73.45% +0.38%
==========================================
Files 358 364 +6
Lines 46956 47882 +926
==========================================
+ Hits 34311 35170 +859
- Misses 12645 12712 +67
Continue to review full report at Codecov.
|
|
@lavdwall ... thanks for posting! Personally, I believe that the ability to access & modify transport polynomials from Python is very useful. Before I get into the details, would you mind adding some unit tests? |
|
Hi @ischoegl , Sorry for the late reply (holidays...). Thanks for your positive response. I will have a look at adding some unit tests in the coming week. What I implemented in the current pull request is really just a get and set of the polynomial coefficients of viscosity, thermal conductivity, binary diffusion coefficients and collision integrals, just as they are available in the C++ core of the code. This is regardless of the transport model implementation that is used, so the user has to be aware of possible differences between CK_Mode = True or CK_Mode = False. And the user needs to be aware of possible discrepancies between the coefficients stored in memory and how those are documented in the doxygen documentation. The documentation is in agreement with textbooks (Kee, and others) but internally in the code the definitions are a bit different it seems. Anyway, I will add test functions to show that set/get works. But maybe on the longer term, the code documentation should be adjusted a bit as well so that everyone can correctly use the access/modify functions without knowledge of the underlying code. |
|
@lavdwall … yes, just checking whether coefficients are written correctly would be adequate. I’m not sure about the documentation; it may be worthwhile to raise an issue first so people with more intricate knowledge can chime in. |
There was a problem hiding this comment.
Hi @lavdwall ... thank you for the PR! While I have quite a few suggestions, this looks mostly good to me. Most of my comments affect details of the implementation.
One caveat here is that I'm not as familiar with some of the changes that affect theory (it's somewhat out of my wheelhouse), but I'm sure that others can fill in those gaps.
|
Linking a recent request for this feature from the User Group. |
|
Hi @lavdwall ... just wanted to briefly check whether you had a chance to look over the review comments? Apart from the change to |
…ification of transport polynomials
|
Hi @ischoegl |
There was a problem hiding this comment.
@lavdwall ... thanks for taking care of the requests. Sorry for asking for one final edit: I hadn't noticed earlier that some of the lines exceed the 88 character limit we try to enforce (see CONTRIBUTING.md style guide). Beyond what I point out below, there's nothing else from my side.
|
Just a quick follow up on this: is there any documentation on the theory behind the current polynomial implementation of the viscosity? |
|
This PR (as well as the associated PR #817) are mostly just dealing with providing a better user interface. The code itself is documented in the C++ include files, which can be accessed by Cantera's doxygen documentation. Here is a link for GasTransport ... while it doesn't provide every detail in terms of theory, things should correspond to standard practice, see reference. |
Some modifications that allow accessing and modifying the gas transport properties polynomial fits, also from the python interface.
This is an addition to the earlier pull request #817 and more specifically, a reply to the suggestion of @ischoegl to make the access functions to the transport polynomials also available in the python interface (see #817 (comment)).
Next to access to the polynomial fits, I also added the possibility to modify the polynomial coefficients. This in C++ as well as Python. Reason: there is literature that reports the polynomial fits to the transport properties explicitly (as function of log(T) or log(Tr)), rather than in terms of the classic L-J parameters. If this is important, the user now has the option to change the polynomial fits for some species or species pairs, after calculating them from the L-J parameters when first reading the *.cti or *.yaml input file. If the option to modify the polynomial fits is not OK for the public repo, I can remove those modification functions, and limit this pull request only to the access functions in the Python interface.
Checklist
scons build&scons test) and unit tests address code coverage