-
-
Notifications
You must be signed in to change notification settings - Fork 346
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
Exposing getDeltaEnthalpies to Matlab interface. #659
Conversation
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.
Seems fine, some typos to fix though 😃
Codecov Report
@@ Coverage Diff @@
## master #659 +/- ##
==========================================
- Coverage 68.49% 68.49% -0.01%
==========================================
Files 368 368
Lines 40027 40030 +3
==========================================
+ Hits 27418 27419 +1
- Misses 12609 12611 +2
Continue to review full report at Codecov.
|
d676b9a
to
eb228f3
Compare
Codecov Report
@@ Coverage Diff @@
## master #659 +/- ##
==========================================
- Coverage 70.78% 70.78% -0.01%
==========================================
Files 373 373
Lines 43447 43450 +3
==========================================
Hits 30754 30754
- Misses 12693 12696 +3
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #659 +/- ##
==========================================
- Coverage 70.78% 70.78% -0.01%
==========================================
Files 373 373
Lines 43451 43450 -1
==========================================
- Hits 30758 30754 -4
- Misses 12693 12696 +3
Continue to review full report at Codecov.
|
% reaction. Units: J/kmol-K | ||
% | ||
|
||
dH = kinetics_get(a.id, 17, 2); |
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.
Sorry, I know I already approved this, but this should probably be dS and the Gibbs should probably be dG
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.
I was about to make the same request! 😁
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.
You guys are so rigid in your style preferences. :)
Fixed now - thanks for the catch!
The general intent here was to enable calculating reaction enthalpies in the Matlab toolbox, as part of the li-ion battery simulations in PR Cantera#563. This required several changes: - Create getDeltaEnthalpies.m in Matlab toolbox/@kinetics, as well as similar methods for Gibbs free energy and entropy of reaction - Add kin_getDelta to kineticsmethods.cpp. - Add getPartialMolarEnthalpies to metalPhase class (it returns all zeros). Note that similar methods are not enabled for the corresponding 'Standard State' methods, for the time being. Mainly because it is difficult for me to envision a significant use case, but also because of some lingering confusion between 'standard' and 'reference' states in Cantera's codebase.
The general intent here was to enable calculating reaction enthalpies in the Matlab toolbox, as part of the li-ion battery simulations in PR #563.
This required several changes: