From a0aead9a4ab0b60a838be5ce2a665835fb603316 Mon Sep 17 00:00:00 2001 From: "Stephen J. Maher" Date: Wed, 18 Sep 2024 14:29:08 +0100 Subject: [PATCH 01/12] adds public method to benders to set free subproblem flag --- src/scip/benders.c | 13 +++++++++++++ src/scip/pub_benders.h | 9 +++++++++ src/scip/reader_sto.c | 9 +++++++++ 3 files changed, 31 insertions(+) diff --git a/src/scip/benders.c b/src/scip/benders.c index 7695d89427..97d815c11b 100644 --- a/src/scip/benders.c +++ b/src/scip/benders.c @@ -6708,6 +6708,19 @@ SCIP_Bool SCIPbendersSubproblemIsEnabled( return benders->subprobenabled[probnumber]; } +/** sets the flag to indicate whether the subproblems must be freed by the Benders' decomposition core. + * NOTE: this is only used if the array of subproblems is freed after initialising the Benders' decomposition. + */ +void SCIPbendersSetFreeSubproblems( + SCIP_BENDERS* benders, /**< Benders' decomposition */ + SCIP_Bool freesubprobs /**< flag to indicate whether the subproblems must be freed by the core */ + ) +{ + assert(benders != NULL); + + benders->freesubprobs = freesubprobs; +} + /** sets a flag to indicate whether the master variables are all set to continuous */ SCIP_RETCODE SCIPbendersSetMastervarsCont( SCIP_BENDERS* benders, /**< Benders' decomposition */ diff --git a/src/scip/pub_benders.h b/src/scip/pub_benders.h index 42029134fc..a9e6ebd4f6 100644 --- a/src/scip/pub_benders.h +++ b/src/scip/pub_benders.h @@ -489,6 +489,15 @@ SCIP_Bool SCIPbendersSubproblemIsEnabled( int probnumber /**< the subproblem number */ ); +/** sets the flag to indicate whether the subproblems must be freed by the Benders' decomposition core. + * NOTE: this is only used if the array of subproblems is freed after initialising the Benders' decomposition. + */ +SCIP_EXPORT +void SCIPbendersSetFreeSubproblems( + SCIP_BENDERS* benders, /**< Benders' decomposition */ + SCIP_Bool freesubprobs /**< flag to indicate whether the subproblems must be freed by the core */ + ); + /** @} */ #ifdef __cplusplus diff --git a/src/scip/reader_sto.c b/src/scip/reader_sto.c index 66d09595ef..6f338fcff4 100644 --- a/src/scip/reader_sto.c +++ b/src/scip/reader_sto.c @@ -2576,6 +2576,11 @@ SCIP_RETCODE buildDecompProblem( getScenarioLowerbound(scip, getScenarioChild(readerdata->scenariotree, i))); } + /* setting the flag to inform the Benders' core that the subproblems need to be freed. This is needed because the + * scenario tree in the reader is freed at the end of reading + */ + SCIPbendersSetFreeSubproblems(benders, TRUE); + /* removing the variable and constraints that were included as part of the core file */ SCIP_CALL( removeCoreVariablesAndConstraints(scip) ); @@ -2681,6 +2686,10 @@ SCIP_RETCODE readSto( stoinputFree(scip, &stoi); SCIPfclose(fp); + /* freeing the scenario tree after the decomposition problem has been built */ + if( readerdata->scenariotree != NULL ) + SCIP_CALL( freeScenarioTree(scip, &readerdata->scenariotree) ); + if( error || retcode != SCIP_OKAY ) return SCIP_READERROR; else From 6ec7ef59cc4c7e1a63e1c7da060b2b783edb0069 Mon Sep 17 00:00:00 2001 From: "Stephen J. Maher" Date: Wed, 18 Sep 2024 15:34:15 +0100 Subject: [PATCH 02/12] adds interface methods to free reader data for COR, TIM and STO --- src/scip/reader_cor.c | 60 +++++++++++++++++++++++++++++++++---------- src/scip/reader_cor.h | 6 +++++ src/scip/reader_sto.c | 51 +++++++++++++++++++++++++++++------- src/scip/reader_sto.h | 6 +++++ src/scip/reader_tim.c | 24 +++++++++++++++-- src/scip/reader_tim.h | 6 +++++ 6 files changed, 129 insertions(+), 24 deletions(-) diff --git a/src/scip/reader_cor.c b/src/scip/reader_cor.c index 07844eb220..2022e76378 100644 --- a/src/scip/reader_cor.c +++ b/src/scip/reader_cor.c @@ -53,6 +53,7 @@ struct SCIP_ReaderData int consnamessize; int nvarnames; int nconsnames; + SCIP_Bool created; SCIP_Bool read; }; @@ -66,14 +67,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 +95,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; } @@ -199,6 +209,9 @@ SCIP_RETCODE SCIPreadCor( readerdata = SCIPreaderGetData(reader); assert(readerdata != NULL); + /* 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 +221,27 @@ SCIP_RETCODE SCIPreadCor( return SCIP_OKAY; } +/** frees the COR reader data */ +SCIP_RETCODE SCIPfreeCorReaderdata( + 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 */ diff --git a/src/scip/reader_cor.h b/src/scip/reader_cor.h index b49bf21782..ffd51deaa5 100644 --- a/src/scip/reader_cor.h +++ b/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 SCIPfreeCorReaderdata( + SCIP* scip /**< the SCIP data structure */ + ); + /* * Interface method for the tim and sto readers */ diff --git a/src/scip/reader_sto.c b/src/scip/reader_sto.c index 6f338fcff4..c44652b37a 100644 --- a/src/scip/reader_sto.c +++ b/src/scip/reader_sto.c @@ -82,6 +82,7 @@ typedef struct StoScenario STOSCENARIO; /** STO reading data */ struct SCIP_ReaderData { + SCIP_Bool created; /**< flag to indicate that the reader data has been created */ SCIP_Bool usebenders; STOSCENARIO* scenariotree; /**< the multi stage scenario tree */ int numscenarios; /**< the total number of scenarios in the scenario tree */ @@ -1011,12 +1012,17 @@ SCIP_RETCODE createReaderdata( assert(scip != NULL); assert(readerdata != NULL); - /* creating the initial scenario */ - SCIP_CALL( createScenarioData(scip, &readerdata->scenariotree) ); + if( !readerdata->created ) + { + readerdata->created = TRUE; + + /* creating the initial scenario */ + SCIP_CALL( createScenarioData(scip, &readerdata->scenariotree) ); - /* setting the scenario name and stage name */ - SCIP_CALL( setScenarioName(scip, readerdata->scenariotree, "ROOT") ); - SCIP_CALL( setScenarioStageName(scip, readerdata->scenariotree, SCIPtimGetStageName(scip, 0)) ); + /* setting the scenario name and stage name */ + SCIP_CALL( setScenarioName(scip, readerdata->scenariotree, "ROOT") ); + SCIP_CALL( setScenarioStageName(scip, readerdata->scenariotree, SCIPtimGetStageName(scip, 0)) ); + } return SCIP_OKAY; } @@ -1031,11 +1037,14 @@ SCIP_RETCODE freeReaderdata( assert(scip != NULL); assert(readerdata != NULL); - /* freeing the scenario tree */ - if( readerdata->scenariotree != NULL ) - SCIP_CALL( freeScenarioTree(scip, &readerdata->scenariotree) ); + if( readerdata->created ) + { + /* freeing the scenario tree */ + if( readerdata->scenariotree != NULL ) + SCIP_CALL( freeScenarioTree(scip, &readerdata->scenariotree) ); - SCIPfreeBlockMemory(scip, &readerdata); + readerdata->created = FALSE; + } return SCIP_OKAY; } @@ -2727,6 +2736,8 @@ SCIP_DECL_READERFREE(readerFreeSto) SCIP_CALL( freeReaderdata(scip, readerdata) ); + SCIPfreeBlockMemory(scip, &readerdata); + return SCIP_OKAY; } @@ -2792,6 +2803,7 @@ SCIP_RETCODE SCIPincludeReaderSto( /* create reader data */ SCIP_CALL( SCIPallocBlockMemory(scip, &readerdata) ); + readerdata->created = FALSE; readerdata->scenariotree = NULL; readerdata->numscenarios = 0; @@ -2864,3 +2876,24 @@ int SCIPstoGetNScenarios( return readerdata->numscenarios; } + +/** frees the COR reader data */ +SCIP_RETCODE SCIPfreeStoReaderdata( + 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; +} diff --git a/src/scip/reader_sto.h b/src/scip/reader_sto.h index 72eb43a3a3..b4fb8014b7 100644 --- a/src/scip/reader_sto.h +++ b/src/scip/reader_sto.h @@ -110,6 +110,12 @@ int SCIPstoGetNScenarios( SCIP* scip /**< SCIP data structure */ ); +/** frees the STO reader data */ +SCIP_EXPORT +SCIP_RETCODE SCIPfreeStoReaderdata( + SCIP* scip /**< the SCIP data structure */ + ); + /** @} */ #ifdef __cplusplus diff --git a/src/scip/reader_tim.c b/src/scip/reader_tim.c index 79e352f35a..fc693fbca4 100644 --- a/src/scip/reader_tim.c +++ b/src/scip/reader_tim.c @@ -329,9 +329,9 @@ void freeReaderdata( SCIPfreeBlockMemoryArray(scip, &readerdata->stagenames, readerdata->nstages); SCIPfreeBlockMemoryArray(scip, &readerdata->stagestartcons, readerdata->nstages); SCIPfreeBlockMemoryArray(scip, &readerdata->stagestartvars, readerdata->nstages); - } - SCIPfreeBlockMemory(scip, &readerdata); + readerdata->read = FALSE; + } } @@ -811,6 +811,8 @@ SCIP_DECL_READERFREE(readerFreeTim) { freeReaderdata(scip, reader); + SCIPfreeBlockMemory(scip, &readerdata); + return SCIP_OKAY; } @@ -913,6 +915,24 @@ SCIP_RETCODE SCIPreadTim( return SCIP_OKAY; } +/** frees the reader data for the tim file */ +SCIP_RETCODE SCIPfreeTimReaderdata( + SCIP* scip /**< the SCIP data structure */ + ) +{ + SCIP_READER* reader; + + assert(scip != NULL); + + reader = SCIPfindReader(scip, READER_NAME); + assert(reader != NULL); + + freeReaderdata(scip, reader); + + return SCIP_OKAY; +} + + /* * Interface methods for the cor and sto files */ diff --git a/src/scip/reader_tim.h b/src/scip/reader_tim.h index 80d0fde7f3..51ce6d74d7 100644 --- a/src/scip/reader_tim.h +++ b/src/scip/reader_tim.h @@ -84,6 +84,12 @@ SCIP_RETCODE SCIPreadTim( /** @} */ +/** frees the reader data for the tim file */ +SCIP_EXPORT +SCIP_RETCODE SCIPfreeTimReaderdata( + SCIP* scip /**< the SCIP data structure */ + ); + /* * Interface methods for the cor and sto files */ From 7a81919402f5e54d85fd2575bb239c260f742bd1 Mon Sep 17 00:00:00 2001 From: "Stephen J. Maher" Date: Fri, 11 Oct 2024 11:28:14 +0100 Subject: [PATCH 03/12] better handling of event handler in Benders --- src/scip/benders.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/scip/benders.c b/src/scip/benders.c index 97d815c11b..831c523e13 100644 --- a/src/scip/benders.c +++ b/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,13 @@ SCIP_RETCODE SCIPbendersDeactivate( BMSfreeMemoryArray(&benders->auxiliaryvars); BMSfreeMemoryArray(&benders->solvestat); BMSfreeMemoryArray(&benders->subproblems); + + /* 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; From b70038dbe0646f1e37e6e953a9a49f898f5c5166 Mon Sep 17 00:00:00 2001 From: "Stephen J. Maher" Date: Fri, 11 Oct 2024 11:29:02 +0100 Subject: [PATCH 04/12] adds methods to free cor, tim and sto reader data --- src/scip/reader_cor.c | 12 +++++++++++- src/scip/reader_sto.c | 8 +++++++- src/scip/reader_tim.c | 31 +++++++++++++++++++++++-------- 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/scip/reader_cor.c b/src/scip/reader_cor.c index 2022e76378..9a11f571f8 100644 --- a/src/scip/reader_cor.c +++ b/src/scip/reader_cor.c @@ -34,6 +34,8 @@ #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 @@ -176,7 +178,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) ); @@ -209,6 +211,14 @@ 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( SCIPfreeCorReaderdata(scip) ); + SCIP_CALL( SCIPfreeTimReaderdata(scip) ); + SCIP_CALL( SCIPfreeStoReaderdata(scip) ); + /* creating the reader data at the start of the instance read */ SCIP_CALL( createReaderdata(scip, readerdata) ); diff --git a/src/scip/reader_sto.c b/src/scip/reader_sto.c index c44652b37a..4dc2b9a80a 100644 --- a/src/scip/reader_sto.c +++ b/src/scip/reader_sto.c @@ -2843,6 +2843,12 @@ SCIP_RETCODE SCIPreadSto( assert(reader != NULL); readerdata = SCIPreaderGetData(reader); + /* before the STO file can be read, the reader data needs to be freed. This is because the reader data stores problem + * data, which needs to be removed before the next instance is read. For most readers, there is no problem data + * stored in the reader data, and hence the reader data doesn't need to be freed. + */ + SCIP_CALL( freeReaderdata(scip, readerdata) ); + retcode = readSto(scip, filename, readerdata); if( retcode == SCIP_PLUGINNOTFOUND ) @@ -2877,7 +2883,7 @@ int SCIPstoGetNScenarios( return readerdata->numscenarios; } -/** frees the COR reader data */ +/** frees the STO reader data */ SCIP_RETCODE SCIPfreeStoReaderdata( SCIP* scip /**< the SCIP data structure */ ) diff --git a/src/scip/reader_tim.c b/src/scip/reader_tim.c index fc693fbca4..8195fad8f7 100644 --- a/src/scip/reader_tim.c +++ b/src/scip/reader_tim.c @@ -37,6 +37,7 @@ #include "scip/pub_reader.h" #include "scip/reader_cor.h" #include "scip/reader_tim.h" +#include "scip/reader_sto.h" #include "scip/scip_mem.h" #include "scip/scip_message.h" #include "scip/scip_numerics.h" @@ -293,17 +294,12 @@ SCIP_RETCODE createReaderdata( static void freeReaderdata( SCIP* scip, /**< SCIP data structure */ - SCIP_READER* reader /**< the reader structure */ + SCIP_READERDATA* readerdata /**< the reader data structure */ ) { - SCIP_READERDATA* readerdata; int i; assert(scip != NULL); - assert(reader != NULL); - - readerdata = SCIPreaderGetData(reader); - assert(readerdata != NULL); /* only free the reader data is a file has been read */ @@ -809,7 +805,15 @@ SCIP_DECL_READERCOPY(readerCopyTim) static SCIP_DECL_READERFREE(readerFreeTim) { - freeReaderdata(scip, reader); + SCIP_READERDATA* readerdata; + + assert(scip != NULL); + assert(reader != NULL); + + readerdata = SCIPreaderGetData(reader); + assert(readerdata != NULL); + + freeReaderdata(scip, readerdata); SCIPfreeBlockMemory(scip, &readerdata); @@ -842,6 +846,13 @@ SCIP_DECL_READERREAD(readerReadTim) return SCIP_OKAY; } + /* when reading the TIM file, it is necessary to free the TIM and STO reader data. This is the STO file generates + * data based on the data from the TIM file. For most readers, there is no problem data stored in the reader data, + * and hence the reader data doesn't need to be freed. + */ + SCIP_CALL( SCIPfreeTimReaderdata(scip) ); + SCIP_CALL( SCIPfreeStoReaderdata(scip) ); + SCIP_CALL( SCIPreadTim(scip, filename, result) ); return SCIP_OKAY; @@ -921,13 +932,17 @@ SCIP_RETCODE SCIPfreeTimReaderdata( ) { SCIP_READER* reader; + SCIP_READERDATA* readerdata; assert(scip != NULL); reader = SCIPfindReader(scip, READER_NAME); assert(reader != NULL); - freeReaderdata(scip, reader); + readerdata = SCIPreaderGetData(reader); + assert(readerdata != NULL); + + freeReaderdata(scip, readerdata); return SCIP_OKAY; } From 3199560dda3d17f6dfe1d605673ac6d7c0ad2b2e Mon Sep 17 00:00:00 2001 From: "Stephen J. Maher" Date: Fri, 1 Nov 2024 11:43:05 +0000 Subject: [PATCH 05/12] moves the free of subproblem array to bendersExitDefault --- src/scip/benders_default.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/scip/benders_default.c b/src/scip/benders_default.c index 048030fcb6..c89ec3c7db 100644 --- a/src/scip/benders_default.c +++ b/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; } From fd34272228b046e7bc5c519acc9ffa6c8019942f Mon Sep 17 00:00:00 2001 From: "Stephen J. Maher" Date: Fri, 1 Nov 2024 11:45:35 +0000 Subject: [PATCH 06/12] resetting data in SCIP_BENDERS on deactivate --- src/scip/benders.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/scip/benders.c b/src/scip/benders.c index 831c523e13..3393799992 100644 --- a/src/scip/benders.c +++ b/src/scip/benders.c @@ -2685,6 +2685,24 @@ SCIP_RETCODE SCIPbendersDeactivate( 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 ) From eb8790a7394044202aa260d132930b2ae9270aa3 Mon Sep 17 00:00:00 2001 From: "Stephen J. Maher" Date: Fri, 1 Nov 2024 11:45:59 +0000 Subject: [PATCH 07/12] adds initialiser for cutefficacy --- src/scip/benderscut_opt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/scip/benderscut_opt.c b/src/scip/benderscut_opt.c index 48f83a8269..0bead5467a 100644 --- a/src/scip/benderscut_opt.c +++ b/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; From 0c7d6dd70877eedaf5838ce8ae4113f95822b7c1 Mon Sep 17 00:00:00 2001 From: "Stephen J. Maher" Date: Fri, 1 Nov 2024 11:46:25 +0000 Subject: [PATCH 08/12] adds SCIPfreeProb to cor reader --- src/scip/reader_cor.c | 2 ++ src/scip/reader_sto.c | 12 +++--------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/scip/reader_cor.c b/src/scip/reader_cor.c index 9a11f571f8..0c0a27acda 100644 --- a/src/scip/reader_cor.c +++ b/src/scip/reader_cor.c @@ -38,6 +38,7 @@ #include "scip/reader_sto.h" #include "scip/scip_mem.h" #include "scip/scip_reader.h" +#include "scip/scip_prob.h" #include #define READER_NAME "correader" @@ -215,6 +216,7 @@ SCIP_RETCODE SCIPreadCor( * 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( SCIPfreeCorReaderdata(scip) ); SCIP_CALL( SCIPfreeTimReaderdata(scip) ); SCIP_CALL( SCIPfreeStoReaderdata(scip) ); diff --git a/src/scip/reader_sto.c b/src/scip/reader_sto.c index 4dc2b9a80a..ab20cfbea3 100644 --- a/src/scip/reader_sto.c +++ b/src/scip/reader_sto.c @@ -2131,6 +2131,9 @@ SCIP_RETCODE getScenarioDecompVar( SCIP_CALL( SCIPaddVar(scip, var) ); + SCIPdebugMessage("Original problem variable <%s> is being duplicated for scenario %d\n", + SCIPvarGetName(var), getScenarioNum(scip, scenario)); + (*scenariovar) = var; (*varadded) = TRUE; } @@ -2585,11 +2588,6 @@ SCIP_RETCODE buildDecompProblem( getScenarioLowerbound(scip, getScenarioChild(readerdata->scenariotree, i))); } - /* setting the flag to inform the Benders' core that the subproblems need to be freed. This is needed because the - * scenario tree in the reader is freed at the end of reading - */ - SCIPbendersSetFreeSubproblems(benders, TRUE); - /* removing the variable and constraints that were included as part of the core file */ SCIP_CALL( removeCoreVariablesAndConstraints(scip) ); @@ -2695,10 +2693,6 @@ SCIP_RETCODE readSto( stoinputFree(scip, &stoi); SCIPfclose(fp); - /* freeing the scenario tree after the decomposition problem has been built */ - if( readerdata->scenariotree != NULL ) - SCIP_CALL( freeScenarioTree(scip, &readerdata->scenariotree) ); - if( error || retcode != SCIP_OKAY ) return SCIP_READERROR; else From 1ea6e0f4b5ff26f4a903931fe0b3e5d03770dea9 Mon Sep 17 00:00:00 2001 From: "Stephen J. Maher" Date: Fri, 1 Nov 2024 11:50:45 +0000 Subject: [PATCH 09/12] reenable 4node1 consecSolve tests --- check/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check/CMakeLists.txt b/check/CMakeLists.txt index 6de8242362..5eb6991c87 100644 --- a/check/CMakeLists.txt +++ b/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" ) From e1f2e1adaf4eb8012db0ee2d5830ba620db64412 Mon Sep 17 00:00:00 2001 From: "Stephen J. Maher" Date: Fri, 1 Nov 2024 11:56:27 +0000 Subject: [PATCH 10/12] removes freesubproblem method --- src/scip/benders.c | 13 ------------- src/scip/pub_benders.h | 9 --------- 2 files changed, 22 deletions(-) diff --git a/src/scip/benders.c b/src/scip/benders.c index 3393799992..9f7f66ebc5 100644 --- a/src/scip/benders.c +++ b/src/scip/benders.c @@ -6737,19 +6737,6 @@ SCIP_Bool SCIPbendersSubproblemIsEnabled( return benders->subprobenabled[probnumber]; } -/** sets the flag to indicate whether the subproblems must be freed by the Benders' decomposition core. - * NOTE: this is only used if the array of subproblems is freed after initialising the Benders' decomposition. - */ -void SCIPbendersSetFreeSubproblems( - SCIP_BENDERS* benders, /**< Benders' decomposition */ - SCIP_Bool freesubprobs /**< flag to indicate whether the subproblems must be freed by the core */ - ) -{ - assert(benders != NULL); - - benders->freesubprobs = freesubprobs; -} - /** sets a flag to indicate whether the master variables are all set to continuous */ SCIP_RETCODE SCIPbendersSetMastervarsCont( SCIP_BENDERS* benders, /**< Benders' decomposition */ diff --git a/src/scip/pub_benders.h b/src/scip/pub_benders.h index a9e6ebd4f6..42029134fc 100644 --- a/src/scip/pub_benders.h +++ b/src/scip/pub_benders.h @@ -489,15 +489,6 @@ SCIP_Bool SCIPbendersSubproblemIsEnabled( int probnumber /**< the subproblem number */ ); -/** sets the flag to indicate whether the subproblems must be freed by the Benders' decomposition core. - * NOTE: this is only used if the array of subproblems is freed after initialising the Benders' decomposition. - */ -SCIP_EXPORT -void SCIPbendersSetFreeSubproblems( - SCIP_BENDERS* benders, /**< Benders' decomposition */ - SCIP_Bool freesubprobs /**< flag to indicate whether the subproblems must be freed by the core */ - ); - /** @} */ #ifdef __cplusplus From 408a2e080ec42c405b992a914600c33985006ab3 Mon Sep 17 00:00:00 2001 From: "Stephen J. Maher" Date: Fri, 1 Nov 2024 12:01:04 +0000 Subject: [PATCH 11/12] updates CHANGELOG --- CHANGELOG | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 52886c14a6..8139a91241 100644 --- a/CHANGELOG +++ b/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. +- SCIPfreeCorReaderdata(), SCIPfreeTimReaderdata() and SCIPfreeStoReaderdata() 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 ------------- From 8694db4a47bc6e712d97fa5de7cab0b9be4d6e1e Mon Sep 17 00:00:00 2001 From: Stefan Vigerske Date: Sat, 9 Nov 2024 17:00:19 +0100 Subject: [PATCH 12/12] rename new API function to follow SCIP style --- CHANGELOG | 2 +- src/scip/reader_cor.c | 8 ++++---- src/scip/reader_cor.h | 2 +- src/scip/reader_sto.c | 2 +- src/scip/reader_sto.h | 2 +- src/scip/reader_tim.c | 6 +++--- src/scip/reader_tim.h | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 8139a91241..755110c0f6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -48,7 +48,7 @@ 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. -- SCIPfreeCorReaderdata(), SCIPfreeTimReaderdata() and SCIPfreeStoReaderdata() for freeing the data for the COR, TIM and +- 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 diff --git a/src/scip/reader_cor.c b/src/scip/reader_cor.c index 0c0a27acda..2dd98e2d77 100644 --- a/src/scip/reader_cor.c +++ b/src/scip/reader_cor.c @@ -217,9 +217,9 @@ SCIP_RETCODE SCIPreadCor( * in the reader data, and hence the data doesn't need to be freed. */ SCIP_CALL( SCIPfreeProb(scip) ); - SCIP_CALL( SCIPfreeCorReaderdata(scip) ); - SCIP_CALL( SCIPfreeTimReaderdata(scip) ); - SCIP_CALL( SCIPfreeStoReaderdata(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) ); @@ -234,7 +234,7 @@ SCIP_RETCODE SCIPreadCor( } /** frees the COR reader data */ -SCIP_RETCODE SCIPfreeCorReaderdata( +SCIP_RETCODE SCIPfreeReaderdataCor( SCIP* scip /**< the SCIP data structure */ ) { diff --git a/src/scip/reader_cor.h b/src/scip/reader_cor.h index ffd51deaa5..a96799b0af 100644 --- a/src/scip/reader_cor.h +++ b/src/scip/reader_cor.h @@ -85,7 +85,7 @@ SCIP_RETCODE SCIPreadCor( /** frees the COR reader data */ SCIP_EXPORT -SCIP_RETCODE SCIPfreeCorReaderdata( +SCIP_RETCODE SCIPfreeReaderdataCor( SCIP* scip /**< the SCIP data structure */ ); diff --git a/src/scip/reader_sto.c b/src/scip/reader_sto.c index ab20cfbea3..bc53d16461 100644 --- a/src/scip/reader_sto.c +++ b/src/scip/reader_sto.c @@ -2878,7 +2878,7 @@ int SCIPstoGetNScenarios( } /** frees the STO reader data */ -SCIP_RETCODE SCIPfreeStoReaderdata( +SCIP_RETCODE SCIPfreeReaderdataSto( SCIP* scip /**< the SCIP data structure */ ) { diff --git a/src/scip/reader_sto.h b/src/scip/reader_sto.h index b4fb8014b7..fc0c789e3d 100644 --- a/src/scip/reader_sto.h +++ b/src/scip/reader_sto.h @@ -112,7 +112,7 @@ int SCIPstoGetNScenarios( /** frees the STO reader data */ SCIP_EXPORT -SCIP_RETCODE SCIPfreeStoReaderdata( +SCIP_RETCODE SCIPfreeReaderdataSto( SCIP* scip /**< the SCIP data structure */ ); diff --git a/src/scip/reader_tim.c b/src/scip/reader_tim.c index 8195fad8f7..56228088cc 100644 --- a/src/scip/reader_tim.c +++ b/src/scip/reader_tim.c @@ -850,8 +850,8 @@ SCIP_DECL_READERREAD(readerReadTim) * data based on the data from the TIM file. For most readers, there is no problem data stored in the reader data, * and hence the reader data doesn't need to be freed. */ - SCIP_CALL( SCIPfreeTimReaderdata(scip) ); - SCIP_CALL( SCIPfreeStoReaderdata(scip) ); + SCIP_CALL( SCIPfreeReaderdataTim(scip) ); + SCIP_CALL( SCIPfreeReaderdataSto(scip) ); SCIP_CALL( SCIPreadTim(scip, filename, result) ); @@ -927,7 +927,7 @@ SCIP_RETCODE SCIPreadTim( } /** frees the reader data for the tim file */ -SCIP_RETCODE SCIPfreeTimReaderdata( +SCIP_RETCODE SCIPfreeReaderdataTim( SCIP* scip /**< the SCIP data structure */ ) { diff --git a/src/scip/reader_tim.h b/src/scip/reader_tim.h index 51ce6d74d7..ab57d987ce 100644 --- a/src/scip/reader_tim.h +++ b/src/scip/reader_tim.h @@ -86,7 +86,7 @@ SCIP_RETCODE SCIPreadTim( /** frees the reader data for the tim file */ SCIP_EXPORT -SCIP_RETCODE SCIPfreeTimReaderdata( +SCIP_RETCODE SCIPfreeReaderdataTim( SCIP* scip /**< the SCIP data structure */ );