New getDeltaRefGibbs and getDeltaRefEntropy methods added to Kinetics.h and InterfaceKinetics.h and .cpp #1
Conversation
Report reaction deltas (of entropy and Gibbs free energy) at Ref Pressure of 1 bar. Changes present in Kinetics.h InterfaceKinetics.h and .cpp
Fix code comment
|
Could you please write a detailed summary for the (pull request) PR at the top and why this PR is needed and describe the changes you introduced. Otherwise the PR would be hard to understand the PR. |
There was a problem hiding this comment.
Hello Shashank,
Thanks for the additions to the code. For consistency, could you please add the methods introduced in InterfaceKientics to GasKinetics as well. The relevant GasKinetics files are GasKinetics.h and GasKinetics.cpp.
I am asking for few minor edits before I merge the code. Please find them in the review.
| virtual void getDeltaRefEntropy(doublereal* deltaS) { | ||
| throw NotImplementedError("Kinetics::getDeltaRefEntropy"); | ||
| } | ||
|
|
There was a problem hiding this comment.
Please explain the functionality of getDeltaRefGibss and getDeltaRefEntropy with comments on top of the methods.
| virtual void getDeltaRefGibbs(doublereal* deltaG) { | ||
| throw NotImplementedError("Kinetics::getDeltaRefGibbs"); | ||
| } | ||
|
|
There was a problem hiding this comment.
For consistency add getDeltaRefEnthalpy as well.
| @@ -106,6 +106,9 @@ class InterfaceKinetics : public Kinetics | |||
| virtual void getDeltaSSEnthalpy(doublereal* deltaH); | |||
| virtual void getDeltaSSEntropy(doublereal* deltaS); | |||
|
|
|||
| virtual void getDeltaRefGibbs(doublereal* deltaG); | |||
| virtual void getDeltaRefEntropy(doublereal* deltaS); | |||
|
|
|||
There was a problem hiding this comment.
For consistency add getDeltaRefEnthalpy as well.
src/kinetics/InterfaceKinetics.cpp
Outdated
| @@ -499,12 +499,33 @@ void InterfaceKinetics::getDeltaSSGibbs(doublereal* deltaGSS) | |||
| // of the solution. | |||
| for (size_t n = 0; n < nPhases(); n++) { | |||
| thermo(n).getStandardChemPotentials(m_mu0.data() + m_start[n]); | |||
| //thermo(n).getGibbs_RT_ref(m_grt.data() + m_start[n]); | |||
There was a problem hiding this comment.
Please delete the commented line added to this PR.
src/kinetics/InterfaceKinetics.cpp
Outdated
| } | ||
|
|
||
| // Use the stoichiometric manager to find deltaG for each reaction. | ||
| //getReactionDelta(m_mu0.data(), deltaGSS); |
There was a problem hiding this comment.
Please delete the newly added commented line
src/kinetics/InterfaceKinetics.cpp
Outdated
| getReactionDelta(m_mu0.data(), deltaGSS); | ||
| } | ||
|
|
||
| void InterfaceKinetics::getDeltaRefGibbs(doublereal * deltaGSS) | ||
| { | ||
| // Get the standard state chemical potentials of the species. This is the |
There was a problem hiding this comment.
Change standard state to reference state in the comment
src/kinetics/InterfaceKinetics.cpp
Outdated
| // Get the standard state chemical potentials of the species. This is the | ||
| // array of chemical potentials at unit activity We define these here as the | ||
| // chemical potentials of the pure species at the temperature and reference | ||
| // pressure of the solution. |
There was a problem hiding this comment.
State the numerical value of the reference pressure as well.
src/kinetics/InterfaceKinetics.cpp
Outdated
| @@ -538,6 +559,22 @@ void InterfaceKinetics::getDeltaSSEntropy(doublereal* deltaS) | |||
| getReactionDelta(m_grt.data(), deltaS); | |||
| } | |||
|
|
|||
| void InterfaceKinetics::getDeltaRefEntropy(doublereal* deltaS) | |||
| { | |||
| // Get the standard state entropy of the species. We define these here as | |||
As per PR mbkumar#1 review, adding the getDeltaRefEnthalpy method definition in the InterfaceKinetics.h file for consistency sake.
Update Kinetics.h based on PR#1. Added comments to the methods getDeltaRefEntropy and getDeltaRefGibbs. Added a new method for getDeltaRefEnthalpy for consistency sake
Deleted commented code. And fix comments to "Reference" state as opposed to "standard" state. Also mentioned reference P of 1 bar.
Formatting change.
I did not update GasKinetics.cpp or GasKinetics.h methods with the new "ref' state methods because, they don't have any of the Delta methods defined (be it at actual state, standard state or reference state). All of the Delta Methods are only re-implemented in the InterfaceKinetics. GasKinetics will use the Base Class
Do I now edit the BulkKinetics methods or override the GasKinetics ones? Do I add all of the the Delta, DeltaSS and DeltaRef methods to the GasKinetics modules (.h and .cpp)? I took care of all of the other minor edits. |
You need to add only DeltaRef methods to the GasKinetics code. The rest (Delta and DeltaSS) will be taken from BulkKinetics. |
I finished doing this just now. I added a "Not Implemented" method for getDeltaRefEnthalpy method as it is giving me an error during the linking process (shown below). We don't need this method so we don't have to define this at this point. It compiles and runs okay with these edits. Please review and close this PR request and the related issue: VlachosGroup/openmkm#50 Error for reference: |
Deleted redundant implementation of virtual methods in the GasKinetics.h and InterfaceKinetics.h header files. Based on mbkumar#1 review feedback.
Okay. Makes sense. Took care of this. It compiles okay now. All yours to review and close. |
Checklist
scons build&scons test) and unit tests address code coverageIf applicable, fill in the issue number this pull request is fixing
Fixes VlachosGroup/openmkm#42, VlachosGroup/openmkm#50
Changes proposed in this pull request
getDeltaRefGibbsandgetDeltaRefEntropygetDeltaRefEnthalpyto the header files Kinetics.h and InterfaceKinetics.h and the implements them in the Kinetics.cpp. The purpose of this method is to estimate reaction Delta entropy and Delta gibbs free energy at the standard stated of each species and also at the reference pressure of 1 bar. These routines are subsequently used in OpenMKM to print the reaction Deltas (S and G) at 1 bar so that it is consistent with the output of the CHEMKIN software package (https://doi.org/10.2172/481906). The reaction deltas is an array with length equal to the number of reactions.