Skip to content

Commit 092ea99

Browse files
Opt-MuccaOpt-Mucca
committed
Rework locks. Add missing lower bound update
1 parent 75b7e7b commit 092ea99

File tree

5 files changed

+52
-23
lines changed

5 files changed

+52
-23
lines changed

‎highs/mip/HighsCutPool.cpp‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ void HighsCutPool::lpCutRemoved(HighsInt cut, bool thread_safe) {
157157
ages_[cut] = 1;
158158
--numLpCuts;
159159
++ageDistribution[1];
160-
if (numLps == 0) {
160+
if (numLps == 1) {
161161
numLps_[cut].fetch_add(-1, std::memory_order_relaxed);
162162
}
163163
}
@@ -180,6 +180,7 @@ void HighsCutPool::performAging() {
180180
propRows.emplace(-1, i);
181181
}
182182
ages_[i] = -1;
183+
++numLpCuts;
183184
}
184185
if (numLps_[i] == 0) {
185186
lpCutRemoved(i);
@@ -250,10 +251,13 @@ void HighsCutPool::separate(const std::vector<double>& sol, HighsDomain& domain,
250251

251252
// if the cut is not violated more than feasibility tolerance
252253
// we skip it and increase its age, otherwise we reset its age
253-
if (!thread_safe) ageDistribution[ages_[i]] -= 1;
254254
bool isPropagated = matrix_.columnsLinked(i);
255-
if (isPropagated && !thread_safe)
256-
propRows.erase(std::make_pair(ages_[i], i));
255+
if (!thread_safe) {
256+
ageDistribution[ages_[i]] -= 1;
257+
if (isPropagated) {
258+
propRows.erase(std::make_pair(ages_[i], i));
259+
}
260+
}
257261
if (double(viol) <= feastol) {
258262
if (thread_safe) continue;
259263
++ages_[i];
@@ -384,6 +388,8 @@ void HighsCutPool::separate(const std::vector<double>& sol, HighsDomain& domain,
384388
break;
385389
}
386390
} else {
391+
// TODO MT: Is this safe for the future? If we copy an LP with a cut
392+
// from a local pool then this is not thread safe
387393
if (getParallelism(p.second, cutset.cutindices[i],
388394
cutpools[cutset.cutpools[i]]) > maxpar) {
389395
discard = true;

‎highs/mip/HighsImplications.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ bool HighsImplications::runProbing(HighsInt col, HighsInt& numReductions) {
365365
substitutions.push_back(substitution);
366366
colsubstituted[implcol] = true;
367367
++numReductions;
368-
} else if (mipsolver.mipdata_->workers.size() <= 1) {
368+
} else if (!mipsolver.mipdata_->parallelLockActive()) {
369369
double lb = std::min(lbDown, lbUp);
370370
double ub = std::max(ubDown, ubUp);
371371

@@ -579,7 +579,7 @@ void HighsImplications::separateImpliedBounds(
579579
if (nextCleanupCall < 0) {
580580
// HighsInt oldNumEntries =
581581
// mipsolver.mipdata_->cliquetable.getNumEntries();
582-
if (mipsolver.mipdata_->workers.size() <= 1)
582+
if (!mipsolver.mipdata_->parallelLockActive())
583583
mipsolver.mipdata_->cliquetable.runCliqueMerging(globaldomain);
584584

585585
// printf("numEntries: %d, beforeMerging: %d\n",
@@ -590,7 +590,7 @@ void HighsImplications::separateImpliedBounds(
590590
// printf("nextCleanupCall: %d\n", nextCleanupCall);
591591
}
592592

593-
if (mipsolver.mipdata_->workers.size() <= 1)
593+
if (!mipsolver.mipdata_->parallelLockActive())
594594
mipsolver.mipdata_->cliquetable.numNeighbourhoodQueries = oldNumQueries;
595595
}
596596

‎highs/mip/HighsLpRelaxation.cpp‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -880,7 +880,7 @@ bool HighsLpRelaxation::computeDualProof(const HighsDomain& globaldomain,
880880

881881
mipsolver.mipdata_->debugSolution.checkCut(inds.data(), vals.data(),
882882
inds.size(), rhs);
883-
if (extractCliques && mipsolver.mipdata_->workers.size() <= 1)
883+
if (extractCliques && !mipsolver.mipdata_->parallelLockActive())
884884
mipsolver.mipdata_->cliquetable.extractCliquesFromCut(
885885
mipsolver, inds.data(), vals.data(), inds.size(), rhs);
886886

@@ -999,7 +999,7 @@ void HighsLpRelaxation::storeDualInfProof() {
999999
dualproofinds.data(), dualproofvals.data(), dualproofinds.size(),
10001000
dualproofrhs);
10011001

1002-
if (mipsolver.mipdata_->workers.size() <= 1) {
1002+
if (!mipsolver.mipdata_->parallelLockActive()) {
10031003
mipsolver.mipdata_->cliquetable.extractCliquesFromCut(
10041004
mipsolver, dualproofinds.data(), dualproofvals.data(),
10051005
dualproofinds.size(), dualproofrhs);
@@ -1444,8 +1444,11 @@ HighsLpRelaxation::Status HighsLpRelaxation::resolveLp(HighsDomain* domain) {
14441444
objsum += roundsol[i] * mipsolver.colCost(i);
14451445

14461446
if (!mipsolver.mipdata_->parallelLockActive()) {
1447-
mipsolver.mipdata_->addIncumbent(roundsol, double(objsum),
1448-
kSolutionSourceSolveLp);
1447+
mipsolver.mipdata_->addIncumbent(
1448+
roundsol, static_cast<double>(objsum), kSolutionSourceSolveLp);
1449+
} else {
1450+
mipsolver.mipdata_->workers[index_].addIncumbent(
1451+
roundsol, static_cast<double>(objsum), kSolutionSourceSolveLp);
14491452
}
14501453
objsum = 0;
14511454
}

‎highs/mip/HighsMipSolver.cpp‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ void HighsMipSolver::run() {
414414
};
415415

416416
auto setParallelLock = [&](bool lock) -> void {
417-
if (mipdata_->workers.size() <= 1) return;
417+
if (!mipdata_->hasMultipleWorkers()) return;
418418
mipdata_->parallel_lock = lock;
419419
for (HighsConflictPool& conflictpool : mipdata_->conflictpools) {
420420
conflictpool.setAgeLock(lock);
@@ -437,7 +437,8 @@ void HighsMipSolver::run() {
437437
};
438438

439439
auto syncPools = [&]() -> void {
440-
if (mipdata_->workers.size() <= 1 || mipdata_->parallelLockActive()) return;
440+
if (!mipdata_->hasMultipleWorkers() || mipdata_->parallelLockActive())
441+
return;
441442
for (HighsInt i = 1; i < mipdata_->conflictpools.size(); ++i) {
442443
mipdata_->conflictpools[i].syncConflictPool(mipdata_->conflictPool);
443444
}
@@ -447,25 +448,25 @@ void HighsMipSolver::run() {
447448
};
448449

449450
auto syncGlobalDomain = [&]() -> void {
450-
if (mipdata_->workers.size() <= 1) return;
451+
if (!mipdata_->hasMultipleWorkers()) return;
451452
for (HighsMipWorker& worker : mipdata_->workers) {
452453
const auto& domchgstack = worker.globaldom_.getDomainChangeStack();
453454
for (const HighsDomainChange& domchg : domchgstack) {
454455
if ((domchg.boundtype == HighsBoundType::kLower &&
455456
domchg.boundval > mipdata_->domain.col_lower_[domchg.column]) ||
456457
(domchg.boundtype == HighsBoundType::kUpper &&
457-
domchg.boundval < mipdata_->domain.col_upper_[domchg.column])) {
458-
mipdata_->domain.changeBound(domchg,
459-
HighsDomain::Reason::unspecified());
460-
}
458+
domchg.boundval < mipdata_->domain.col_upper_[domchg.column])) {
459+
mipdata_->domain.changeBound(domchg,
460+
HighsDomain::Reason::unspecified());
461+
}
461462
}
462463
}
463464
};
464465

465466
auto resetDomains = [&]() -> void {
466467
search.resetLocalDomain();
467468
mipdata_->domain.clearChangedCols();
468-
if (mipdata_->workers.size() <= 1) return;
469+
if (!mipdata_->hasMultipleWorkers()) return;
469470
for (HighsInt i = 1; i < mip_search_concurrency; i++) {
470471
mipdata_->domains[i] = mipdata_->domain;
471472
mipdata_->workers[i].globaldom_ = mipdata_->domains[i];
@@ -716,6 +717,21 @@ void HighsMipSolver::run() {
716717
}
717718
}
718719

720+
// Handle case where all nodes have been pruned (and lb hasn't been updated
721+
// due to parallelism)
722+
// TODO MT: Change the if statement
723+
// if (mipdata_->hasMultipleWorkers() && num_search_indices == 0) {
724+
if (mipdata_->hasMultipleWorkers() &&
725+
(num_search_indices == 0 || search_indices[0] != 0)) {
726+
double prev_lower_bound = mipdata_->lower_bound;
727+
mipdata_->lower_bound = std::min(mipdata_->upper_bound,
728+
mipdata_->nodequeue.getBestLowerBound());
729+
if (!submip && (mipdata_->lower_bound != prev_lower_bound))
730+
mipdata_->updatePrimalDualIntegral(
731+
prev_lower_bound, mipdata_->lower_bound, mipdata_->upper_bound,
732+
mipdata_->upper_bound);
733+
}
734+
719735
if (mipdata_->checkLimits()) {
720736
analysis_.mipTimerStop(kMipClockNodePrunedLoop);
721737
return std::make_pair(true, false);
@@ -926,7 +942,7 @@ void HighsMipSolver::run() {
926942
break;
927943
}
928944

929-
if (!mipdata_->parallelLockActive()) {
945+
if (!mipdata_->hasMultipleWorkers()) {
930946
HighsInt numPlungeNodes = mipdata_->num_nodes - plungestart;
931947
if (numPlungeNodes >= 100) break;
932948

@@ -937,7 +953,7 @@ void HighsMipSolver::run() {
937953
if (!backtrack_plunge) break;
938954
}
939955

940-
if (!mipdata_->parallelLockActive()) assert(search.hasNode());
956+
if (!mipdata_->hasMultipleWorkers()) assert(search.hasNode());
941957

942958
analysis_.mipTimerStart(kMipClockPerformAging2);
943959
for (HighsConflictPool& conflictpool : mipdata_->conflictpools) {
@@ -952,7 +968,7 @@ void HighsMipSolver::run() {
952968
mipdata_->workers[i].search_ptr_->flushStatistics();
953969
}
954970
mipdata_->printDisplayLine();
955-
if (mipdata_->parallelLockActive()) break;
971+
if (mipdata_->hasMultipleWorkers()) break;
956972
// printf("continue plunging due to good estimate\n");
957973
} // while (true)
958974
analysis_.mipTimerStop(kMipClockDive);

‎highs/mip/HighsMipSolverData.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,11 @@ struct HighsMipSolverData {
258258
const HighsInt user_solution_callback_origin);
259259

260260
bool parallelLockActive() const {
261-
return (parallel_lock && workers.size() <= 1);
261+
return (parallel_lock && hasMultipleWorkers());
262+
}
263+
264+
bool hasMultipleWorkers() const {
265+
return workers.size() > 1;
262266
}
263267
};
264268

0 commit comments

Comments
 (0)