search

Aalto-CFD / DLBFoam

DLBFoam: An open-source dynamic load balancing model for fast reacting flow simulations in OpenFOAM. https://doi.org/10.1016/j.cpc.2021.108073, https://doi.org/10.1063/5.0077437
GNU General Public License v3.0
76 stars 27 forks source link

Change of state variable OFv10 onwards #24

Closed blttkgl closed 2 years ago

blttkgl commented 2 years ago

I know v1.1_OF10 branch is still in dev, but I just wanted to leave this issue here as a reminder that OpenFOAM switched the main state variable in chemistry class to mass fraction in v10 (https://github.com/OpenFOAM/OpenFOAM-10/commit/4f0dfc3bdf09ae6cf9caf0b806af6fabf3002186), which needs to be reflected within DLBFoam, especially v1.0 without pyJac. This includes refactoring the "getVariables" approach to differentiate between mass fraction and concentration state variables between pyjac and no-pyjac approaches, as well as the updateReactionRate function that differs between two chemistry models.

blttkgl commented 2 years ago

Please check the pull request https://github.com/Aalto-CFD/DLBFoam/pull/25 for fixing this issue.

moreff commented 2 years ago

@blttkgl It is interesting that the tests didn't capture this... Did you do any validations?

blttkgl commented 2 years ago

I didn't take a very close look at the tests, but concentration and mass fraction are by definition very close concepts to one another, so I wouldn't be surprised if the tests were green even with this issue.

Currently the unittests are passed without any issues. If you checked the changes I made in the pull request, I only changed the principle variable to be mass fraction by removing the concentration/massfraction difference between loadBalancedChemistryModel and pyJacLoadBalancedChemistryModel. Apart from these I did not make any validations, but I encourage that it is done before it is merged with the main.

moreff commented 2 years ago

Ok, so this change only affects loadBalancedChemistryModel, since pyJacLoadBalancedChemistryModel is anyway using mass fraction. Currently, validations cover only pyJacLoadBalancedChemistryModel, thus tests did not show any problems. I did some validations and with your fixes, everything works fine