search

coin-or / Cgl

Cut Generator Library
Other
24 stars 14 forks source link

Cgl012cut.cpp is not threadsafe #63

Open tosttost opened 3 years ago

tosttost commented 3 years ago

In Cgl012cut.cpp line 3289:

static double max_score_ever = ZERO; /* maximum score of a violated cut during
                    the whole cutting plane procedure */

this global variable is used and updated during separation, so multiple instances of Cgl012cut running in parallel (or probably sequential within the same process) will influence each other.

Note that I have no idea if

Is there anyone around that knows about the internals of Cgl012cut?

tkralphs commented 3 years ago

Looks to me like it is intentional that the variable is global. I guess it would be algorithmically valid to make it a field in the sense that the value it gets will be valid, just maybe not "as good" as the value it would get it if it were global. One of the limitations of the modular design of Cgl is that it's not easy to maintain this kind of global information, but I'm not sure how best to modify this. Interestingly, there is a commit back in 2014 to "make zerohalf threadsafe" 2b40662993d6f77855591a53e485de42578aab65. Maybe @jjforrest can comment.

jjhforrest commented 3 years ago

As parallel cbc is normally non-deterministic, this does not matter bug-wise as it just means cuts may or may not get generated (but they will be valid ones).

But it would be tidier - I don't know who knows about internals of Cgl012Cut - but it is quite likely that I was last person to modify it - so I can modify code if nobody else volunteers.

John Forrest

On 13/05/2021 15:14, tosttost wrote:

In Cgl012cut.cpp line 3289:

|static double max_score_ever = ZERO; / maximum score of a violated cut during the whole cutting plane procedure / |

this global variable is used and updated during separation, so multiple instances of Cgl012cut running in parallel (or probably sequential within the same process) will influence each other.

Note that I have no idea if

  • what effects this might have on Cbc
  • it is algorithmically valid to turn that global variable into a field of Cgl012cut (which would be easy to do)

Is there anyone around that knows about the internals of Cgl012cut?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/coin-or/Cgl/issues/63, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWJYHGSLPNBPH3IRK5EDOTTNPNCRANCNFSM442UCWDA.

tosttost commented 3 years ago

Thanks for the explanation. If only less cuts but no invalid cuts are separated, the severity of this issue is reduced significantly.

It might be an exotic use case but there are definitely people who use cbc as a library to solve multiple MIP within the same process. Writing problems into files and calling the cbc executable is not always viable:

Therefore thread safety is not only relevant in terms of "./cbc foobar.mps -threads 42 -solve".

Mutable global variables are not reset. Therefore even if the problems are solved sequentially within the same process they will influence each other.