Added switch to change mixing rule for MW relaxation time. Fixing issue #310 - #311
Added switch to change mixing rule for MW relaxation time. Fixing issue #310#311capriatim wants to merge 15 commits into
Conversation
|
I've tested this and it seems to work as intended. I have a few comments on the code, which I will post shortly. Nothing major though. |
| // 1: Gnoffo, 1989 | ||
| static int i_model = 0; | ||
|
|
||
| void setMillikanWhiteModel(int model) |
There was a problem hiding this comment.
Can be const int model
|
|
||
| // 0: orginal, default | ||
| // 1: Gnoffo, 1989 | ||
| static int i_model = 0; |
There was a problem hiding this comment.
I would prefer enumerators here to make it more clear which is which, but not necessary.
| const Eigen::Map<const Eigen::ArrayXd> Xh( | ||
| thermo.X()+(thermo.hasElectrons() ? 1 : 0), thermo.nHeavy()); | ||
|
|
||
| const Eigen::Map<const Eigen::ArrayXd> Yh( |
There was a problem hiding this comment.
Yh is only used for imodel == 0, so I would move this inside the if statement to avoid unnecessary work when imodel == 1.
| const double tau_park = 1.0/(ni * ci * m_data.limitingCrossSection(thermo.T())); | ||
| double tau = 0; | ||
|
|
||
| if (i_model == 0) { |
There was a problem hiding this comment.
While this approach works, it might be slow in practice because the if on imodel will have to be checked every time the Millikan-White term is needed. In a CFD code, where this is called for every cell on every iteration, this could be quite costly.
One alternative is to adopt the approach taken throughout M++, and use an object-oriented approach. Make an abstract base class for the Millikan-White model, with two concrete classes for the original and Gnoffo models. Then you can use a factory to construct the desired model.
| /// Set Millikan–White relaxation-time model | ||
| /// 0 = original (default) | ||
| /// 1 = Gnoffo (1989) | ||
| void setMillikanWhiteModel(int model); |
|
|
||
| // 0: orginal, default | ||
| // 1: Gnoffo, 1989 | ||
| static int i_model = 0; |
There was a problem hiding this comment.
It would be best to avoid floating static int for imodel. Could this be stored inside the MillikanWhiteModelData class instead?
| Mixture mix(opts); | ||
|
|
||
| const double thetav = 3408.464; | ||
| setMillikanWhiteModel(1); // Use Gnoffo (1989) |
There was a problem hiding this comment.
Great that this still passes. Can you add a test for the original model?
|
Hi @jbscoggi, |
2 participants
No description provided.