Open wandadars opened 3 weeks ago
Attention: Patch coverage is 6.43275%
with 320 lines
in your changes missing coverage. Please review.
Project coverage is 75.47%. Comparing base (
026c5c7
) to head (d505b61
). Report is 48 commits behind head on main.
Files | Patch % | Lines |
---|---|---|
src/transport/HighPressureGasTransport.cpp | 4.51% | 317 Missing :warning: |
...clude/cantera/transport/HighPressureGasTransport.h | 25.00% | 3 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
One thing that I noticed is that, while we may utilize the critical-properties.yaml file in the cubic equation of state to obtain critical properties, the input YAML file needs to also have its own "critical-paramters' section with the acentric-factor defined. This is the current way that the transport properties class obtains the quantity.
Also, the difference between using the tabulated values directly from the YAML data versus making a query to the cubic eos asking it for the mixture critical properties Tc, Pc, Vc, does seem to have a strong effect. In the Poling book, example 9-1 for Sulfur Dioxide, the tabulated critical volume is 122 cm^3/mol, but the Peng-Robinson equation of state gives a critical volume of 97.6 cm^3/mol. So we have a 20% difference in the critical properties between tabulated values and EOS-derived values.
With regards to diffusion coefficients at high pressure, the Funazukuri, 1992 paper method that Poling mentions in the book might be worth checking out.
One thing that might need to be checked is whether a user has provided an acentric-factor, because if they don't have it included in the "critical-properties" section in the YAML file, a default value of 0 is used quietly I think. That would be undesirable. So, even if a user provides the "acentric-factor" in the "equation-of-state" section, it won't be used by the transport model and a value of 0 is used.
I'm also working on checking the case of what happens if a user only has a subset of species defined as having critical properties (for cases where critical properties of the other mixture gases may not be known). I want to make sure that the mixing rules don't cause any FPEs when the EOS is asked for the critical values of Pc, Tc, Vc for a species that is ideal. I know for the PREOS, you can provide a=0 and b=0 to recover ideal behavior. I just want to make sure it all works out in that situation.
@speth Is there a general guiding philosphy established for what types of statements should be in loglevel print statements? I have code blocks in the high pressure transport that I'm using for evaluating the operation of the model, but they're just cout blocks and I have them commented out. I was thinking that having them accessible via a loglevel might be useful.
It looks like most of these blocks are just printing out parameter values on input to a function, right? I'd be inclined to just delete them. I think once you have this working, they will have served their purpose and won't be needed.
This implementation is equation-heavy with references to the Poling book. I am trying to find a balance between having all equations replicated in the documentation versus referring to the book. Any thoughts on how I should proceed here?
That's a good question. I guess the ideal would be to include all the equations that are needed to explain the calculations. That would provide users with the ability to understand what Cantera's doing without needing to consult a somewhat difficult to obtain reference, and makes it possible to more easily verify that the implementation corresponds to the equations. On the other hand, that may be an excessive number of equations and effort, and anything at all would be an improvement over the current version of this class.
One issue I have is with regards to a unit test. I want to try and compare the result to something from the Poling book, but the critical volume estimated from the cubic EOS is 20% off of the tabulated/known value, so that generates an error in the viscosity or thermal conductivity.
@speth I think this is mostly in a working state now. I know you guys have tons of other things to review. I think this is ready if anyone wants to start the process of reviewing the changes.
The general approach that I have taken with these Lucas and Chung method is that often in the Poling book, the mixture-based calculation is identical to the pure-species equation with the only difference being that mixture-average values of the parameters in the equations are used. So I implemented "blind" methods that are the pure-species versions of the low pressure viscosity and high pressure viscosity methods and then I left the calculation of the mixture values and the passing of those mixture values to the "blind" methods up to the main method (viscosity()).
I put the mixture values into a struct so they could be cleanly computed in one place and passed around if needed.
We have an updated Lucas method (The HighPressureGas class) and a new Chung method (ChungHighPressureGas) class.
Both of these classes use the same Takahasi method, so I moved that into a separate standalone function that both classes could call. I also have a standalone Neufeld collision integral function. This is used in the Poling book for estimating the Omega parameter in the viscosity equations. I know Cantera has it's own calculation of that value, but I wasn't sure how to go about getting it properly, so I put the standalone function in for now.
I tried to keep the comments clear and inside the methods, make reference to the equations in the Poling book (pdfs can be found online of the book).
When looking into the Takahashi result, we can not interpolate the coefficients from the paper as it produces wild results that look nothing like the plots. I do believe that the authors used some sort of curve fit the data points in the table in order to generate the plots from their paper. For us, using the results from the table presented in the Takahashi paper produces piecewise linear segments between the data points. That's the best that can be done I think, without us introducing our own smooth curve fit to the data in the table, which I've always been taught to not do.
This PR is a WIP
Changes proposed in this pull request
I will add examples when the primary coding is complete. The original class had no tests or examples and was marked for deprecation, so I'm hoping that this effort will save it from deletion.
Checklist
scons build
&scons test
) and unit tests address code coverage