Infinite loop in VCS equilibrium solver

[GoogleCodeExporter]

What steps will reproduce the problem?
Using the current trunk version, run:

    python samples/python/equilibrium/adiabatic_flame/adiabatic.py

What is the expected output? What do you see instead?
Program gets stuck in an infinite loop inside the VCS equilibrium solver

This bug appears to have been introduced somewhere in the 
liquidTransportDevelop branch, and was brought into trunk the last time that 
branch was merged (r1115). The sample appears to run correctly in the last 
revision of the scons branch before it was merged into trunk (r1112).

The example works correctly using the MultiPhaseEquil solver.

Original issue reported on code.google.com by yarmond on 12 Sep 2012 at 9:27

[GoogleCodeExporter]

The issue is still in cantera-2.0.2 and cantera-2.1.0r2 (with python 2.7 
modules). 

Original comment by ischo...@gmail.com on 9 Aug 2013 at 7:58

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

In response to comment #1, here's a patch that solves the problem for r2516.

It modifies the src/equil/vcs_solve_TP.cpp file in two ways:

1. It slightly modifies the way deleted species are re-introduced at the end of 
the vcs_solve_TP function, reverting back to what was done in revision 1112. In 
my experience, this solves most of the infinite loop issues but not all of them.

2. It removes the last call to L_RETURN_BLOCK When the maximum number of vcs 
iterations (MAXITER) is reached. Instead, the solver then returns directly -1 
(error). This solves the infinite loop problem for good.

I tried contacting Harry Moffat about the issue to ask if, in his opinion, 
those modification might have unforeseen consequences, but I still have to hear 
back from him. From a purely 'numerical' point of view, the second modification 
would by itself be sufficient to get rid of this issue. Nevertheless, given 
that the first modification fixes most of the problems, I was keen on keeping 
it in the patch.

Charles

Original comment by chdoi...@gmail.com on 12 Aug 2013 at 9:16

Attachments:

• issue113.patch

[GoogleCodeExporter]

Thanks for supplied patch. I agree that the second change is a good backstop to 
eliminate the possible infinite loops.

I'm less sure about the other change. I like it in that it fixes the problem 
without triggering the error condition of hitting the iteration limit. On the 
other hand, I'm not sure what problem the bit of code that's being removed was 
supposed to be solving in the first place. 

Unless Harry wants to explain what that code does and supply a different patch 
that achieves the same result, I'm inclined to accept this patch.

Original comment by yarmond on 15 Aug 2013 at 3:13

[GoogleCodeExporter]

Original comment by yarmond on 15 Aug 2013 at 3:14

• Changed state: Started

[GoogleCodeExporter]

I agree that skipping the L_RETURN_BLOCK when the maximum number of iterations 
is reached seems reasonable, and will eliminate the infinite loop. 

The other modification, by (in my experience) decreasing the probability that 
another iteration is performed after a first call to the L_RETURN_BLOCK only 
improves the probability that the number of maximum iterations will not be 
reached, increasing the robustness of the solver. It is therefore not as 
critical as the first one.

Finally you're fully right that it would be useful to know why the changes that 
are reverted back with the first part of the patch were introduced in the 
vcs_deltag function in the first place. This would allow one to judge whether 
the (apparent) advantages of reverting to the old version of the code come at 
the expense of something else. 

Original comment by chdoi...@gmail.com on 21 Aug 2013 at 8:32

[GoogleCodeExporter]

This issue was closed by revision r2525.

Original comment by yarmond on 23 Aug 2013 at 5:43

• Changed state: Fixed