Clean up: remove code in if(false) branch
This logic was added in commit https://github.com/OPM/opm-simulators/commit/035d2166414d97c7f45e5e42496c162cc20bccb4 (PR https://github.com/OPM/opm-simulators/pull/4895). @steink: Can you have a look at this?
jenkins build this please
I am fine with removing this if @steink agrees. But I have lately tested an approach that stops iterating (i.e. just give up) if it stagnates multiple times. This potentially gives some speedup but more testing is needed. Is is hard to add a communication step here to fix the parallel issue? You dont need to do it, I just want to know in case I pick this idea up at a later stage. And what about the similar code in iterateWellEqWithControl(...) does that create problems with MPI
I am fine with removing this if @steink agrees. But I have lately tested an approach that stops iterating (i.e. just give up) if it stagnates multiple times. This potentially gives some speedup but more testing is needed. Is is hard to add a communication step here to fix the parallel issue? You dont need to do it, I just want to know in case I pick this idea up at a later stage. And what about the similar code in iterateWellEqWithControl(...) does that create problems with MPI
We can also leave this code in here if it is needed for future purposes, also there is no communication needed here, I just noticed this when I was looking through MultisegmentWell_impl.hpp, but all good.
I have to admit I don't recall this was part of https://github.com/OPM/opm-simulators/pull/4895, but I'm fine with deleting it. In general I'm sceptical to accepting wells with very loose convergence criteria because well convergence problems is typically a symptom of something else than just that the well is difficult to solve.
I have to admit I don't recall this was part of #4895, but I'm fine with deleting it. In general I'm sceptical to accepting wells with very loose convergence criteria because well convergence problems is typically a symptom of something else than just that the well is difficult to solve.
@steink Thanks :) then this can be merged safely?
I just added a draft PR that start using this part of the code https://github.com/OPM/opm-simulators/pull/6020 I think some code that detects stagnation / oscillation is a good idea.
I am fine with removing this if @steink agrees. But I have lately tested an approach that stops iterating (i.e. just give up) if it stagnates multiple times. This potentially gives some speedup but more testing is needed. Is is hard to add a communication step here to fix the parallel issue? You dont need to do it, I just want to know in case I pick this idea up at a later stage. And what about the similar code in iterateWellEqWithControl(...) does that create problems with MPI
You mean this part here? https://github.com/OPM/opm-simulators/blob/master/opm/simulators/wells/MultisegmentWell_impl.hpp#L1539, so far I have not seen problems with MPI at this part
I just added a draft PR that start using this part of the code #6020 I think some code that detects stagnation / oscillation is a good idea.
Thanks!
Closing this, since the issue was addressed in https://github.com/OPM/opm-simulators/pull/6020.