Merge branch '3359-smps-variable-release-bug' into 'master'

Resolves bug variable not released when consecutively reading SMPS files

Closes #3359

See merge request integer/scip!3562

CHANGELOG

@@ -48,6 +48,8 @@ Interface changes
 - Renamed XML functions to avoid name clash with libxml2 by adding "SCIP": SCIPxmlProcess(), SCIPxmlNewNode(), SCIPxmlNewAttr(), SCIPxmlAddAttr(), SCIPxmlAppendChild(), SCIPxmlFreeNode(), SCIPxmlShowNode(), SCIPxmlGetAttrval(), SCIPxmlFirstNode(), SCIPxmlNextNode(), SCIPxmlFindNode(), SCIPxmlFindNodeMaxdepth(), SCIPxmlNextSibl(), SCIPxmlPrevSibl(), SCIPxmlFirstChild(), SCIPxmlLastChild(), SCIPxmlGetName(), SCIPxmlGetLine(), SCIPxmlGetData(), SCIPxmlFindPcdata().
 - SCIPincludePresolImplint() to include the new implied integer presolver
 - SCIPnetmatdecCreate() and SCIPnetmatdecFree() for creating and deleting a network matrix decomposition. SCIPnetmatdecTryAddCol() and SCIPnetmatdecTryAddRow() are used to add columns and rows of the matrix to the decomposition. SCIPnetmatdecContainsRow() and SCIPnetmatdecContainsColumn() check if the decomposition contains the given row or columns. SCIPnetmatdecRemoveComponent() can remove connected components from the decomposition. SCIPnetmatdecCreateDiGraph() can be used to expose the underlying digraph. SCIPnetmatdecIsMinimal() and SCIPnetmatdecVerifyCycle() check if certain invariants of the decomposition are satisfied and are used in tests.
+- SCIPfreeReaderdataCor(), SCIPfreeReaderdataTim() and SCIPfreeReaderdataSto() for freeing the data for the COR, TIM and
+  STO readers respectively. These readers are all used when reading an SMPS instance.
 
 ### Changes in preprocessor macros
 
@@ -92,6 +94,10 @@ Build system
 Fixed bugs
 ----------
 
+- fixed bug related to unreleased data for the Benders' decomposition framework. When reading an SMPS file and applying
+  Benders' decomposition, data is created that was not correctly released. Also, data within the Benders' decomposition
+  framework was not appropriately reset. The data is now released/reset as expected.
+
 Miscellaneous
 -------------
 

check/CMakeLists.txt

@@ -242,7 +242,7 @@ set(instances_consecSolve
   "instances/MINLP/meanvarxsc.lp\;14.3692321148754"
   "instances/SOS/sparse2.lp\;26.0"
   "instances/Cardinality/atm_5_25_1.cip\;+1.40606905557936e+05"
-#  "instances/Stochastic/4node1.smps\;480.9"
+  "instances/Stochastic/4node1.smps\;480.9"
   "instances/SAT/bart10.shuffled.cnf\;0"
   )
 

src/scip/benders.c

@@ -2609,14 +2609,17 @@ SCIP_RETCODE SCIPbendersActivate(
          SCIP_CALL( SCIPpqueueInsert(benders->subprobqueue, benders->solvestat[i]) );
       }
 
-      /* adding an eventhandler for updating the lower bound when the root node is solved. */
-      eventhdlrdata = (SCIP_EVENTHDLRDATA*)benders;
-
-      /* include event handler into SCIP */
-      SCIP_CALL( SCIPincludeEventhdlrBasic(set->scip, &eventhdlr, NODESOLVED_EVENTHDLR_NAME, NODESOLVED_EVENTHDLR_DESC,
-            eventExecBendersNodesolved, eventhdlrdata) );
-      SCIP_CALL( SCIPsetEventhdlrInitsol(set->scip, eventhdlr, eventInitsolBendersNodesolved) );
-      assert(eventhdlr != NULL);
+      if( SCIPsetFindEventhdlr(set, NODESOLVED_EVENTHDLR_NAME) == NULL )
+      {
+         /* adding an eventhandler for updating the lower bound when the root node is solved. */
+         eventhdlrdata = (SCIP_EVENTHDLRDATA*)benders;
+
+         /* include event handler into SCIP */
+         SCIP_CALL( SCIPincludeEventhdlrBasic(set->scip, &eventhdlr, NODESOLVED_EVENTHDLR_NAME, NODESOLVED_EVENTHDLR_DESC,
+               eventExecBendersNodesolved, eventhdlrdata) );
+         SCIP_CALL( SCIPsetEventhdlrInitsol(set->scip, eventhdlr, eventInitsolBendersNodesolved) );
+         assert(eventhdlr != NULL);
+      }
    }
 
    return SCIP_OKAY;
@@ -2636,6 +2639,7 @@ SCIP_RETCODE SCIPbendersDeactivate(
 
    if( benders->active )
    {
+      SCIP_EVENTHDLR* eventhdlr;
       int nsubproblems;
 
       nsubproblems = SCIPbendersGetNSubproblems(benders);
@@ -2680,6 +2684,31 @@ SCIP_RETCODE SCIPbendersDeactivate(
       BMSfreeMemoryArray(&benders->auxiliaryvars);
       BMSfreeMemoryArray(&benders->solvestat);
       BMSfreeMemoryArray(&benders->subproblems);
+
+      benders->ncalls = 0;
+      benders->ncutsfound = 0;
+      benders->ntransferred = 0;
+
+      benders->naddedsubprobs = 0;
+      benders->nconvexsubprobs = 0;
+      benders->nnonlinearsubprobs = 0;
+      benders->subprobscreated = FALSE;
+      benders->freesubprobs = FALSE;
+      benders->masterisnonlinear = FALSE;
+
+      benders->nstrengthencuts = 0;
+      benders->nstrengthencalls = 0;
+      benders->nstrengthenfails = 0;
+
+      benders->npseudosols = 0;
+      benders->feasibilityphase = FALSE;
+
+      /* dropping the event from the node solved event handler */
+      eventhdlr = SCIPsetFindEventhdlr(set, NODESOLVED_EVENTHDLR_NAME);
+      if( eventhdlr != NULL && SCIPsetGetStage(set) >= SCIP_STAGE_INITSOLVE )
+      {
+         SCIP_CALL( SCIPdropEvent(set->scip, SCIP_EVENTTYPE_NODESOLVED, eventhdlr, NULL, -1) );
+      }
    }
 
    return SCIP_OKAY;

src/scip/benders_default.c

@@ -80,7 +80,7 @@ struct SCIP_BendersData
    SCIP_VAR***           subproblemvars;     /**< the subproblem variables corresponding to master problem variables */
    int                   nmastervars;        /**< the number of variables in the master problem */
    int                   nsubproblems;       /**< the number of subproblems */
-   SCIP_Bool             created;            /**< flag to indicate that the Benders' decomposition Data was created */
+   SCIP_Bool             subprobscreated;    /**< flag to indicate that the Benders' decomposition Data was created */
    SCIP_Bool             subprobscopied;     /**< were the subproblems copied during the SCIP copy */
    SCIP_Bool             mappingcreated;     /**< flag to indicate whether the variable mapping has been created */
 };
@@ -114,7 +114,7 @@ SCIP_RETCODE createBendersData(
    for( i = 0; i < nsubproblems; i++ )
       (*bendersdata)->subproblems[i] = subproblems[i];
 
-   (*bendersdata)->created = TRUE;
+   (*bendersdata)->subprobscreated = TRUE;
 
    return SCIP_OKAY;
 }
@@ -303,7 +303,6 @@ static
 SCIP_DECL_BENDERSFREE(bendersFreeDefault)
 {  /*lint --e{715}*/
    SCIP_BENDERSDATA* bendersdata;
-   int i;
 
    assert(scip != NULL);
    assert(benders != NULL);
@@ -316,19 +315,7 @@ SCIP_DECL_BENDERSFREE(bendersFreeDefault)
    assert(bendersdata->subproblemvars == NULL);
    assert(bendersdata->subvartomastervar == NULL);
    assert(bendersdata->mastervartosubindex == NULL);
-   if( bendersdata->created )
-   {
-      /* if the subproblems were copied, then the copy needs to be freed */
-      if( bendersdata->subprobscopied )
-      {
-         for( i = bendersdata->nsubproblems - 1; i >= 0; i-- )
-         {
-            SCIP_CALL( SCIPfree(&bendersdata->subproblems[i]) );
-         }
-      }
-
-      SCIPfreeBlockMemoryArray(scip, &bendersdata->subproblems, bendersdata->nsubproblems);
-   }
+   assert(bendersdata->subproblems == NULL);
 
    SCIPfreeBlockMemory(scip, &bendersdata);
 
@@ -387,6 +374,23 @@ SCIP_DECL_BENDERSEXIT(bendersExitDefault)
       SCIPhashmapFree(&bendersdata->mastervartosubindex);
    }
 
+   assert(bendersdata->subproblemvars == NULL);
+   assert(bendersdata->subvartomastervar == NULL);
+   assert(bendersdata->mastervartosubindex == NULL);
+   if( bendersdata->subprobscreated )
+   {
+      /* if the subproblems were copied, then the copy needs to be freed */
+      if( bendersdata->subprobscopied )
+      {
+         for( i = bendersdata->nsubproblems - 1; i >= 0; i-- )
+         {
+            SCIP_CALL( SCIPfree(&bendersdata->subproblems[i]) );
+         }
+      }
+
+      SCIPfreeBlockMemoryArray(scip, &bendersdata->subproblems, bendersdata->nsubproblems);
+   }
+
    return SCIP_OKAY;
 }
 

src/scip/benderscut_opt.c

@@ -316,6 +316,7 @@ SCIP_RETCODE computeMIRForOptimalityCut(
    SCIP_CALL( SCIPaggrRowAddCustomCons(masterprob, aggrrow, rowinds, rowvals, nvars, -lhs, 1.0, 1, FALSE) );
 
    /* calculating a flow cover for the optimality cut */
+   cutefficacy = 0.0;
    SCIP_CALL( SCIPcalcFlowCover(masterprob, sol, TRUE, 0.9999, FALSE, aggrrow, cutcoefs, cutrhs, cutinds, cutnnz,
          &cutefficacy, NULL, &cutislocal, &cutsuccess) );
    (*success) = cutsuccess;

src/scip/reader_cor.c

@@ -34,8 +34,11 @@
 #include "scip/pub_reader.h"
 #include "scip/reader_cor.h"
 #include "scip/reader_mps.h"
+#include "scip/reader_tim.h"
+#include "scip/reader_sto.h"
 #include "scip/scip_mem.h"
 #include "scip/scip_reader.h"
+#include "scip/scip_prob.h"
 #include <string.h>
 
 #define READER_NAME             "correader"
@@ -53,6 +56,7 @@ struct SCIP_ReaderData
    int                   consnamessize;
    int                   nvarnames;
    int                   nconsnames;
+   SCIP_Bool             created;
    SCIP_Bool             read;
 };
 
@@ -66,14 +70,18 @@ SCIP_RETCODE createReaderdata(
    assert(scip != NULL);
    assert(readerdata != NULL);
 
-   readerdata->read = FALSE;
-   readerdata->nvarnames = 0;
-   readerdata->nconsnames = 0;
-   readerdata->varnamessize = SCIP_DEFAULT_ARRAYSIZE;
-   readerdata->consnamessize = SCIP_DEFAULT_ARRAYSIZE;
+   if( !readerdata->created )
+   {
+      readerdata->created = TRUE;
+      readerdata->read = FALSE;
+      readerdata->nvarnames = 0;
+      readerdata->nconsnames = 0;
+      readerdata->varnamessize = SCIP_DEFAULT_ARRAYSIZE;
+      readerdata->consnamessize = SCIP_DEFAULT_ARRAYSIZE;
 
-   SCIP_CALL( SCIPallocBlockMemoryArray(scip, &readerdata->varnames, readerdata->varnamessize) );
-   SCIP_CALL( SCIPallocBlockMemoryArray(scip, &readerdata->consnames, readerdata->consnamessize) );
+      SCIP_CALL( SCIPallocBlockMemoryArray(scip, &readerdata->varnames, readerdata->varnamessize) );
+      SCIP_CALL( SCIPallocBlockMemoryArray(scip, &readerdata->consnames, readerdata->consnamessize) );
+   }
 
    return SCIP_OKAY;
 }
@@ -90,14 +98,19 @@ SCIP_RETCODE freeReaderdata(
    assert(scip != NULL);
    assert(readerdata != NULL);
 
-   for( i = readerdata->nvarnames - 1; i >= 0; i-- )
-      SCIPfreeBlockMemoryArray(scip, &readerdata->varnames[i], strlen(readerdata->varnames[i]) + 1);
+   if( readerdata->created )
+   {
+      for( i = readerdata->nvarnames - 1; i >= 0; i-- )
+         SCIPfreeBlockMemoryArray(scip, &readerdata->varnames[i], strlen(readerdata->varnames[i]) + 1);
 
-   for( i = readerdata->nconsnames - 1; i >= 0; i-- )
-      SCIPfreeBlockMemoryArray(scip, &readerdata->consnames[i], strlen(readerdata->consnames[i]) + 1);
+      for( i = readerdata->nconsnames - 1; i >= 0; i-- )
+         SCIPfreeBlockMemoryArray(scip, &readerdata->consnames[i], strlen(readerdata->consnames[i]) + 1);
 
-   SCIPfreeBlockMemoryArray(scip, &readerdata->consnames, readerdata->consnamessize);
-   SCIPfreeBlockMemoryArray(scip, &readerdata->varnames, readerdata->varnamessize);
+      SCIPfreeBlockMemoryArray(scip, &readerdata->consnames, readerdata->consnamessize);
+      SCIPfreeBlockMemoryArray(scip, &readerdata->varnames, readerdata->varnamessize);
+   }
+
+   readerdata->created = FALSE;
 
    return SCIP_OKAY;
 }
@@ -166,7 +179,7 @@ SCIP_RETCODE SCIPincludeReaderCor(
 
    /* create reader data */
    SCIP_CALL( SCIPallocBlockMemory(scip, &readerdata) );
-   SCIP_CALL( createReaderdata(scip, readerdata) );
+   readerdata->created = FALSE;
 
    /* include reader */
    SCIP_CALL( SCIPincludeReaderBasic(scip, &reader, READER_NAME, READER_DESC, READER_EXTENSION, readerdata) );
@@ -199,6 +212,18 @@ SCIP_RETCODE SCIPreadCor(
    readerdata = SCIPreaderGetData(reader);
    assert(readerdata != NULL);
 
+   /* when the COR file is read, it is necessary to free the reader data from the COR, TIM and STO readers. This is
+    * because the COR file is the base file for the TIM and STO files. For most readers, there is no problem data stored
+    * in the reader data, and hence the data doesn't need to be freed.
+    */
+   SCIP_CALL( SCIPfreeProb(scip) );
+   SCIP_CALL( SCIPfreeReaderdataCor(scip) );
+   SCIP_CALL( SCIPfreeReaderdataTim(scip) );
+   SCIP_CALL( SCIPfreeReaderdataSto(scip) );
+
+   /* creating the reader data at the start of the instance read */
+   SCIP_CALL( createReaderdata(scip, readerdata) );
+
    SCIP_CALL( SCIPreadMps(scip, reader, filename, result, &readerdata->varnames, &readerdata->consnames,
          &readerdata->varnamessize, &readerdata->consnamessize, &readerdata->nvarnames, &readerdata->nconsnames) );
 
@@ -208,6 +233,27 @@ SCIP_RETCODE SCIPreadCor(
    return SCIP_OKAY;
 }
 
+/** frees the COR reader data */
+SCIP_RETCODE SCIPfreeReaderdataCor(
+   SCIP*                 scip                /**< the SCIP data structure */
+   )
+{
+   SCIP_READER* reader;
+   SCIP_READERDATA* readerdata;
+
+   assert(scip != NULL);
+
+   reader = SCIPfindReader(scip, READER_NAME);
+   assert(reader != NULL);
+
+   readerdata = SCIPreaderGetData(reader);
+   assert(readerdata != NULL);
+
+   SCIP_CALL( freeReaderdata(scip, readerdata) );
+
+   return SCIP_OKAY;
+}
+
 /*
  * Interface method for the tim and sto readers
  */

src/scip/reader_cor.h

@@ -83,6 +83,12 @@ SCIP_RETCODE SCIPreadCor(
    SCIP_RESULT*          result              /**< pointer to store the result of the file reading call */
    );
 
+/** frees the COR reader data */
+SCIP_EXPORT
+SCIP_RETCODE SCIPfreeReaderdataCor(
+   SCIP*                 scip                /**< the SCIP data structure */
+   );
+
 /*
  * Interface method for the tim and sto readers
  */