Added multifluid_ecs_mutant #102
Conversation
There was a problem hiding this comment.
I think the unit conversions cancel out, right? So please remove them. Otherwise, I think it looks fine after the comments are resolved.
One last thing I noticed: please add at least one test to the catch tests as an integration test so we can catch any memory issues if present
|
|
||
| auto tc_func = pow(molefraction[0], 2.0) * Tc[0] + pow(molefraction[1], 2.0) * Tc[1] + 2.0 * molefraction[0] * molefraction[1] * \ | ||
| (p00 + p10 * delta + p01 * tau + p20 * delta * delta + p02 * tau * tau + p11 * delta * tau) * tc_scale; | ||
| return forceeval(tc_func); |
| auto tau = tc_scale / temperature; | ||
| auto delta = density / dc_scale; | ||
|
|
||
| auto vc_ = pow(molefraction[0], 2.0) * vc[0] * 1E3 + pow(molefraction[1], 2.0) * vc[1] * 1E3 + 2.0 * molefraction[0] * molefraction[1] * \ |
There was a problem hiding this comment.
Why are these unit conversions in here? teqp is supposed to operate with base SI unless specified explicitly.
There was a problem hiding this comment.
Its an intern reduction for the polynomial to avoid high coefficients, the numver also could be a fixed thing, but I picked the critical constants.
There was a problem hiding this comment.
I stick with my original idea: stick with base SI everywhere, one less things to get confused by. The coefficients can be whatever; does it really make a difference if the degree of the polynomials is relatively small?
|
|
||
| public: | ||
|
|
||
| Eigen::ArrayXd Tc, vc; |
There was a problem hiding this comment.
Some doxygen docs here on what is in Tc and vc would be nice
|
|
||
| auto vc_ = pow(molefraction[0], 2.0) * vc[0] * 1E3 + pow(molefraction[1], 2.0) * vc[1] * 1E3 + 2.0 * molefraction[0] * molefraction[1] * \ | ||
| (p00 + p10 * delta + p01 * tau + p20 * delta * delta + p02 * tau * tau + p11 * delta * tau) * vc_scale * 1E3; | ||
| auto dc_func = 1E3 * (1.0 / vc_); |
There was a problem hiding this comment.
again unit conversion, please use base SI
| auto p11 = dr_coeffs(4, 0) + molefraction[0] * dr_coeffs(4, 1) + molefraction[0] * molefraction[0] * dr_coeffs(4, 2); | ||
| auto p02 = dr_coeffs(5, 0) + molefraction[0] * dr_coeffs(5, 1) + molefraction[0] * molefraction[0] * dr_coeffs(5, 2); | ||
| auto dc_scale = 1.0/(0.125* pow( pow(vc[0],1.0/3.0) + pow(vc[1],1.0/3.0),3.0)); | ||
| auto vc_scale = (0.125 * pow(pow(vc[0], 1.0 / 3.0) + pow(vc[1], 1.0 / 3.0), 3.0)); |
There was a problem hiding this comment.
why is this not 1/dc_scale?
| auto tau = tc_scale / temperature; | ||
| auto delta = density / dc_scale; | ||
|
|
||
| auto vc_ = pow(molefraction[0], 2.0) * vc[0] * 1E3 + pow(molefraction[1], 2.0) * vc[1] * 1E3 + 2.0 * molefraction[0] * molefraction[1] * \ |
There was a problem hiding this comment.
I stick with my original idea: stick with base SI everywhere, one less things to get confused by. The coefficients can be whatever; does it really make a difference if the degree of the polynomials is relatively small?
| namespace teqp { | ||
|
|
||
| /*! | ||
| Implementation of the polynomial extended corresponding states (pECS) mixture model \n |
There was a problem hiding this comment.
This will do - I will tidy it up after I merge
Added a polynomial based extended corresponding states mixture model.
So far its working only for binary mixtures.