search

mutationpp / Mutationpp

The MUlticomponent Thermodynamic And Transport library for IONized gases in C++
GNU Lesser General Public License v3.0
106 stars 58 forks source link

Fixed while loop in EquilStateModel::setState #103

Closed athmargaritis closed 4 years ago

athmargaritis commented 4 years ago

Attempt to fix issue #102

jbscoggi commented 4 years ago

Hey @athmargaritis, thanks for the fix! The only problem I foresee is that rhoe (as you said in #102) can be negative, so simply adding a tolerance to that will only shift the zero point, rather than moving the curve into the strictly positive regime. Since we only care about absolute values, I would suggest to divide by (|rho_e| + tol), which should have the desired effect.

I noticed that you also changed the maximum iterations to 100 from 50. Was this completely necessary for your application or is this a leftover from testing?

athmargaritis commented 4 years ago

Hi @jbscoggi, I feel stupid for missing my own explanation. Indeed, the std::abs solves the problem correctly. As I mentioned though, the rho=0 exactly pretty much never happens, so the endless loop is the usual bug outcome.

Going from 50 to 100 max iterations was probably not necessary, feel free to reject this change. I just changed this first and then made all other corrections and was reluctant to go back to 50 just in case. In my experience though, if it can't converge in 50 it probably won't converge in 100.

Will add one more commit adding the std::abs and switching back to 50. Will let you know if testing shows that 100 was necessary in the end.

jbscoggi commented 4 years ago

Sounds good @athmargaritis. I'll wait for the tests to pass and merge. Thanks!