ClpEventHandler not pointing to correct model

[pauldaylightning]

In ClpSimplex::initialSolve, the "this" ClpSimplex pointer has an eventhandler with a model_ member variable that looks like it should be pointing back to "this" - but it does not, at least at the start of the initialSolve function (i.e. line "ClpSimplex * model2 = this"). (It appears that some, but not all, ClpSimplex constructors/copies/etc do not properly initialise eventhandler model_). My call stack at this point is: CbcModel::initialSolve() -> OsiClpSolverInterface::initialSolve() -> ClpSimplex::initialSolve(ClpSolve& options).

This means that any calls to eventHandler() will not have the correct model to look up. E.g. near start of initialSolve: "eventHandler()->event(ClpEventHandler::presolveStart)".

In many initialSolve code paths, this problem corrects itself through either hitting a line with "model2->eventHandler()->setSimplex(model2)" or "model2 = pinfo->presolvedModel(...)" where this latter function creates a new ClpSimplex object where the eventhandler model_ member variable is set correctly on construction (i.e. copy constructor ClpSimplex::ClpSimplex(const ClpSimplex &rhs, int scalingMode) does initialise model_, but other constructors do not).

But other code paths are broken. For example: model2 starts off pointing to "this" (with bad eventhandler). Then during presolve, model2 points to a new ClpSimplex object (with good eventhandler). But then we hit code that decides to switch model2 back to "this" (code block line ~1040 with comment "We may be better off using original (but if dual leave because of bounds)"), which has bad eventhandler. From now on my eventhandler code never gets a pointer to the correct model -> none of my eventhandling code works.

I have applied a hacky workaround patch that seems to fix my particular occurence of this problem (see below), but it seems a more correct solution would be to fix at the constructor level (and/or in eventhandler clone())? I am not familiar enough with the code base to know what is safe to do here.

Hacky workaround (in ClpSimplex::initialSolve at around line 896): ClpSimplex * model2 = this; model2->eventHandler()->setSimplex(model2); // <---- added this line

If you need any further details please let me know. Thanks.

[jjhforrest]

Updated ClpSimplex.cpp - hopefully fixes

[pauldaylightning]

Thanks. It doesn't look like this is quite enough for my situation. I can still see the eventhandler model_ getting out of sync. It looks like it happens around this code in OsiClpSolverInterface::initialSolve():

    // Check (in branch and bound)
    if ((specialOptions_&1024)==0) {
      solver = new ClpSimplex(true);
      deleteSolver=true;
      solver->borrowModel(*modelPtr_);  // **<-- problem in here??**
      // See if user set factorization frequency
      // borrowModel does not move
      solver->factorization()->maximumPivots(userFactorizationFrequency);
    } else {

After the highlighted line above "solver" now has an eventhandler pointing to the wrong model. Perhaps the "borrow constructors" need something similar to your other recent ClpSimplex constructor code changes? I.e. in ClpSimplex::borrowModel(ClpSimplex & otherModel) and ClpSimplex::borrowModel(ClpModel & otherModel)? Or should it be done in eventhandler clone? (Again, I am not familiar enough with the code to work out exactly what is safe to do).

Thanks a lot for your help!

[svigerske]

The changes that @jjhforrest did only made it into the stable/1.17 branch. Maybe you did not get them? I just merged them into trunk. Note sure when the mirror to github will kick in.

[pauldaylightning]

I did get and try those changes from stable/1.17. So bug still exists.

I can get a fix for my situation by doing: Add following line at end of functions ClpSimplex::borrowModel(ClpSimplex & otherModel) and ClpSimplex::borrowModel(ClpModel & otherModel)

eventHandler_->setSimplex(this);

But not sure if this is best or appropriate solution. Thanks.

[jjhforrest]

Put in asserts to check but seems Ok since last update

[svigerske]

@jjhforrest commited bfda5e57c0cf to trunk/master. I'm not sure it is supposed to fix this issue. Does it for you?

[pauldaylightning]

I still have the model pointer mismatch problem in borrowModel (see previous comment).