Conversation
akifcorduk
added
bug
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds time-limit propagation and handling across the solver: new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cpp/src/dual_simplex/cuts.cpp (2)
179-191:⚠️ Potential issue | 🟠 MajorReturn value mismatch when breaking early on time limit.
If the time-limit guard at line 180 triggers a
break, only a prefix ofbest_cuts_is appended to the outputbest_cutsCSR matrix, but line 191 still returnsbest_cuts_.size()— the full count from the scoring phase. Any caller using the return value as the number of rows in the output matrix will be wrong.Consider returning
best_cuts.m(the actual number of rows appended) instead:Proposed fix
- return static_cast<i_t>(best_cuts_.size()); + return best_cuts.m;
2544-2704:⚠️ Potential issue | 🔴 CriticalAdd critical validation for
remove_cuts_statusreturn value in the caller (branch_and_bound.cpp:2304).The return value from
remove_cuts()is assigned toremove_cuts_statusbut never checked by the caller. In contrast,add_cuts_statusis properly validated at lines 2189–2195. Ifremove_cuts()returns -2 (timeout) at line 2696 (after LP mutations at lines 2667–2674 but before basis refactoring completes), the LP state remains partially mutated and inconsistent. Sincemutex_original_lp_is released at line 2319, other threads can then access a corruptedoriginal_lp_.Add validation immediately after the mutex is unlocked:
mutex_original_lp_.unlock(); if (remove_cuts_status == -2) { solver_status_ = mip_status_t::TIME_LIMIT; set_final_solution(solution, root_objective_); return solver_status_; } else if (remove_cuts_status != 0) { settings_.log.printf("Failed to remove cuts\n"); return mip_status_t::NUMERICAL; }
🤖 Fix all issues with AI agents
In `@cpp/src/dual_simplex/branch_and_bound.cpp`:
- Around line 2285-2289: The current combined condition calls set_final_solution
twice because stop_for_time_limit() already invokes set_final_solution(solution,
root_objective_); change the logic so stop_for_time_limit() is evaluated first
and, if it returns true, set solver_status_ = mip_status_t::TIME_LIMIT and
return immediately without calling set_final_solution again; only call
set_final_solution from this block when scratch_status ==
lp_status_t::TIME_LIMIT (or otherwise when stop_for_time_limit() did not handle
it). Ensure changes reference stop_for_time_limit(), set_final_solution(...),
solver_status_, mip_status_t::TIME_LIMIT, scratch_status and
lp_status_t::TIME_LIMIT.
🧹 Nitpick comments (3)
cpp/src/dual_simplex/branch_and_bound.cpp (2)
2086-2086: Many adjacentstop_for_time_limit()checks; consider consolidating.Several pairs of checks are only a few lines apart with no meaningful work in between (e.g., Lines 2122 vs 2125, Lines 2231 vs 2244). Each check calls
toc(), which is inexpensive but the pattern is visually noisy. You could keep one check at the top of each logical block (generate → score → select → add → presolve → dual_phase2 → remove) and remove the interstitial ones without materially affecting responsiveness.This is purely a readability observation—the current form is correct—so feel free to ignore if you prefer the extra safety margin.
Also applies to: 2104-2104, 2116-2116, 2122-2122, 2125-2125, 2134-2134, 2167-2167, 2184-2184, 2220-2220, 2231-2231, 2244-2244, 2265-2265, 2273-2273, 2301-2301, 2320-2320
2189-2193: Exposeadd_cutsreturn value constants in the public API header.The implementation defines
constexpr i_t time_limit_status = -2locally withincuts.cpp, but the caller inbranch_and_bound.cppmust hardcode the magic sentinel-2to check for time limits (line 2189). Since the constant is inaccessible from the header, the API contract is implicit and fragile.Define and export status constants (or an enum) in
cuts.hppso callers can reference named constants instead of hardcoding-2. This clarifies the return value contract and prevents maintenance issues if the sentinel value changes.cpp/src/dual_simplex/cuts.hpp (1)
455-484: Growing parameter lists onadd_cuts/remove_cuts— consider a context struct.Both
add_cuts(12 parameters) andremove_cuts(15 parameters) are accumulating more arguments. While the current addition is justified, the long parameter lists make call sites hard to read and easy to mis-order. A lightweightcut_context_tstruct bundling the LP state, basis, solution vectors, and timing info would improve readability and make future additions non-breaking.Not blocking—just a suggestion for when these signatures next need to change.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
build.sh (1)
393-398:⚠️ Potential issue | 🟠 MajorRegression:
-n(no-install) flag is broken whenUSE_NVCC_JOBSERVERis OFF.Previously,
hasArg -nwas used as the condition, so the make path handled no-install builds. Now the condition isUSE_NVCC_JOBSERVER, but theelsebranch unconditionally passes--target ${INSTALL_TARGET}. When-nis given,INSTALL_TARGETis empty (line 217), so the unquoted expansion vanishes andcmakeinterprets the next token (e.g.-vor-j<N>) as the target name, causing a build failure.Compare with the
libmps_parserblock (lines 353–357) which correctly branches on-nto omit--target.Proposed fix
if [ "${USE_NVCC_JOBSERVER}" = "ON" ]; then # Manual make invocation to start its jobserver make ${JFLAG} -C "${REPODIR}/cpp" LIBCUOPT_BUILD_DIR="${LIBCUOPT_BUILD_DIR}" VERBOSE_FLAG="${VERBOSE_FLAG}" PARALLEL_LEVEL="${PARALLEL_LEVEL}" ninja-build else - cmake --build "${LIBCUOPT_BUILD_DIR}" --target ${INSTALL_TARGET} ${VERBOSE_FLAG} ${JFLAG} + if [ -n "${INSTALL_TARGET}" ]; then + cmake --build "${LIBCUOPT_BUILD_DIR}" --target ${INSTALL_TARGET} ${VERBOSE_FLAG} ${JFLAG} + else + cmake --build "${LIBCUOPT_BUILD_DIR}" ${VERBOSE_FLAG} ${JFLAG} + fi fi
cpp/src/dual_simplex/cuts.cpp
Outdated
| f_t start_time) | ||
|
|
||
| { | ||
| constexpr i_t time_limit_status = -2; |
There was a problem hiding this comment.
-2 is the return status for CONCURRENT_LIMIT. We should probably not use that to avoid confusion.
| @@ -95,14 +95,19 @@ f_t cut_pool_t<i_t, f_t>::cut_orthogonality(i_t i, i_t j) | |||
| } | |||
There was a problem hiding this comment.
I don't think all these timing checks are necessary. In my experiments most of the time cut generation goes very quickly and I added work limits to catch places where it does take a while.
There was a problem hiding this comment.
Here are the results on MIPLIB. The cut gen time shows the sum of cut generation time over all cut passes. So the time for any cut one put pass can be 1/10 of that. The largest time for cut generation is 35 seconds.
model gap gomory mir cg knapsack cut gen cut dual status solve time
neos-5049753-cuanza 1.26e-01 1454 1 6 0 35.1 69.1 Not Solved 300.13
neos-3988577-wolgan inf 8 0 0 0 3.6 2.1 Not Solved 300.03
var-smallemery-m6j6 2.35e-02 - - - - 0.2 0.0 Not Solved 300.03
uccase9 2.20e-01 220 0 0 0 0.8 1.6 Not Solved 300.01
leo1 6.59e-02 9 25 3 0 0.2 0.1 Not Solved 300.00
uct-subprob 1.42e-01 24 0 0 0 0.2 0.2 Not Solved 300.00
30n20b8 6.67e-01 35 44 1 0 0.3 0.3 Not Solved 300.00
air05 9.50e-05 1 0 0 0 0.1 0.0 Optimal 1.74
app1-1 0.00e+00 145 74 3 0 0.9 0.1 Optimal 2.30
h80x6320d 1.35e-01 320 0 0 0 0.8 0.1 Not Solved 300.01
neos-4387871-tavua 5.60e-01 141 0 0 0 0.7 0.5 Not Solved 300.00
radiationm40-10-02 1.30e-04 605 9 266 0 0.4 0.7 Not Solved 300.00
50v-10 6.25e-02 91 2 1 0 0.1 0.0 Not Solved 300.00
academictimetablesmall 1.00e+00 101 72 11 0 1.4 1.9 Not Solved 300.01
app1-2 2.53e+00 56 31 0 0 9.6 0.7 Not Solved 302.24
highschool1-aigio 1.00e+00 34 34 34 34 7.7 123.0 Not Solved 300.00
neos-4413714-turia 1.36e-01 626 83 0 0 0.4 3.0 Not Solved 300.05
rail01 inf 14 14 14 14 21.8 18.1 Not Solved 300.00
assign1-5-8 9.16e-02 47 0 0 0 0.0 0.0 Not Solved 300.00
atlanta-ip 1.23e-01 19 0 0 0 1.9 0.6 Not Solved 300.03
b1c1s1 3.23e-01 536 63 0 0 0.8 0.2 Not Solved 300.00
hypothyroid-k1 1.73e-02 38 0 0 0 1.8 2.1 Not Solved 302.63
neos-4532248-waihi 9.28e-01 102 0 16 0 12.8 5.4 Not Solved 301.50
rail02 4.30e-01 - - - - 0.0 0.0 Not Solved 300.41
bab2 inf 120 0 55 0 20.2 8.2 Not Solved 300.18
bab6 1.16e+00 79 0 64 0 5.8 1.5 Not Solved 300.07
beasleyC3 3.14e-01 717 111 28 0 0.4 0.6 Not Solved 300.01
ic97_potential 1.54e-02 48 32 6 0 0.0 0.0 Not Solved 300.00
neos-4647030-tutaki 2.22e-04 800 6 6 0 0.2 0.6 Not Solved 300.03
rail507 6.88e-03 1 0 0 0 1.5 0.1 Not Solved 300.01
binkar10_1 5.41e-03 12 22 0 0 0.0 0.0 Not Solved 300.00
blp-ar98 3.96e-02 65 72 39 0 1.6 0.3 Not Solved 300.01
blp-ic98 4.75e-02 17 27 14 0 0.5 0.1 Not Solved 300.01
icir97_tension 1.88e-03 0 1 0 0 0.0 0.0 Not Solved 300.00
neos-4722843-widden 1.57e-02 2454 0 0 0 16.6 5.9 Not Solved 300.03
ran14x18-disj-8 4.04e-02 1 0 0 0 0.0 0.0 Not Solved 300.00
bnatt400 inf 37 142 6 0 0.2 0.3 Not Solved 300.00
bnatt500 inf 36 173 7 0 0.3 0.4 Not Solved 300.00
bppc4-08 2.66e-02 4 2 0 0 0.0 0.0 Not Solved 300.00
cbs-cta 0.00e+00 171 0 0 0 0.2 0.1 Optimal 2.21
irish-electricity 6.96e-02 3469 6 0 0 11.9 8.6 Not Solved 300.03
neos-4738912-atrato 3.48e-03 64 49 1 0 0.2 0.1 Not Solved 300.00
rd-rplusc-21 9.99e-01 45 1 0 0 1.0 0.0 Not Solved 303.61
irp 1.00e-04 5 0 0 0 0.1 0.0 Optimal 17.59
istanbul-no-cutoff 0.00e+00 23 317 0 0 2.4 1.0 Optimal 18.26
brazil3 9.61e-01 4 4 4 4 0.5 294.2 Not Solved 300.00
buildingenergy 1.68e-03 213 0 0 0 2.4 0.4 Not Solved 300.03
chromaticindex1024-7 2.50e-01 - - - - 0.0 0.0 Not Solved 301.07
neos-4763324-toguru 3.30e-01 71 0 0 0 9.8 1.2 Not Solved 300.04
reblock115 5.51e-03 57 0 0 0 0.7 0.4 Not Solved 300.00
rmatr100-p10 8.00e-05 4 0 0 0 0.5 0.1 Optimal 4.27
chromaticindex512-7 2.50e-01 2 0 0 0 3.8 1.4 Not Solved 301.49
k1mushroom 2.61e-01 0 0 0 0 9.7 85.3 Not Solved 311.87
cmflsp50-24-8-8 2.15e-02 16 0 0 0 1.9 0.4 Not Solved 300.01
CMS750_4 7.94e-03 1451 0 0 0 0.2 0.0 Not Solved 300.01
neos-4954672-berkel 2.87e-01 278 176 0 0 0.1 0.1 Not Solved 300.00
rmatr200-p5 2.09e-01 1 0 0 0 1.1 0.1 Not Solved 300.20
co-100 4.70e-01 163 1672 57 0 6.2 3.2 Not Solved 300.04
lectsched-5-obj 4.44e-01 1932 651 441 0 7.4 2.0 Not Solved 300.02
cod105 5.24e-01 1 0 0 0 0.3 49.9 Not Solved 304.56
comp07-2idx 3.33e-01 31 0 1 0 1.3 1.1 Not Solved 300.01
graphdraw-domain 3.20e-01 103 30 0 0 0.1 0.0 Not Solved 300.00
rocI-4-11 1.19e+00 114 82 2 0 0.1 0.0 Not Solved 300.00
comp21-2idx 9.82e-01 44 0 0 0 0.6 0.5 Not Solved 300.00
cost266-UUE 5.57e-02 94 54 0 0 0.1 0.2 Not Solved 300.00
cryptanalysiskb128n5obj14 inf 146 126 92 0 7.5 9.5 Not Solved 300.01
neos-5052403-cygnet 1.37e-02 - - - - 0.0 0.0 Not Solved 300.09
rocII-5-11 1.09e+00 57 66 3 0 0.2 0.0 Not Solved 300.00
cryptanalysiskb128n5obj16 inf 205 115 220 0 6.5 10.4 Not Solved 300.02
leo2 1.00e-01 12 27 1 0 0.4 0.1 Not Solved 300.01
csched007 1.41e-01 38 59 1 0 0.0 0.0 Not Solved 300.00
csched008 2.84e-02 6 33 0 0 0.0 0.0 Not Solved 300.00
dano3_3 0.00e+00 - - - - 0.0 0.0 Optimal 103.88
decomp2 0.00e+00 1 1 1 1 0.1 0.0 Optimal 0.77
drayage-100-23 0.00e+00 28 13 0 0 0.0 0.0 Optimal 1.14
neos-5093327-huahum 1.90e-01 276 0 0 0 0.6 0.7 Not Solved 300.02
dano3_5 1.00e-04 3 0 0 0 0.0 0.3 Optimal 103.83
rococoB10-011000 3.18e-01 76 137 13 0 0.8 0.2 Not Solved 300.00
cvs16r128-89 2.87e-01 6 0 0 0 0.9 0.3 Not Solved 300.01
lotsize 6.57e-01 827 2012 1 0 0.6 0.3 Not Solved 300.00
eil33-2 9.10e-05 1 0 0 0 0.0 0.0 Optimal 7.43
drayage-25-23 1.28e-02 21 18 5 0 0.0 0.0 Not Solved 300.00
enlight_hard 0.00e+00 - - - - 0.0 0.0 Optimal 300.00
neos-5104907-jarama 2.15e-01 497 0 43 0 30.4 168.2 Not Solved 300.61
ex10 0.00e+00 - - - - 0.0 0.0 Optimal 300.00
ex9 0.00e+00 - - - - 0.0 0.0 Optimal 300.00
dws008-01 7.34e-01 21 5 3 0 0.3 0.0 Not Solved 300.00
rococoC10-001000 8.16e-02 107 216 14 0 0.6 0.1 Not Solved 300.00
mad 1.00e+00 1 7 0 0 0.0 0.0 Not Solved 300.00
eilA101-2 1.24e-01 1 0 0 0 0.6 0.2 Not Solved 300.02
neos-5107597-kakapo 5.66e-01 980 45 10 0 0.0 0.0 Not Solved 300.00
exp-1-500-5-5 1.27e-01 596 3 0 0 0.1 0.0 Not Solved 300.00
fhnw-binpack4-48 0.00e+00 1784 216 0 0 0.1 0.0 Optimal 0.76
fast0507 6.78e-03 2 0 0 0 1.5 0.1 Not Solved 300.01
roi2alpha3n4 1.29e-01 26 184 0 0 0.5 0.4 Not Solved 300.03
map10 4.54e-02 1 0 0 0 1.0 0.1 Not Solved 300.09
fastxgemm-n2r6s0t2 4.48e-01 3 0 0 0 0.0 0.0 Not Solved 300.00
neos-5114902-kasavu 3.65e-01 2 2 2 2 1.0 49.5 Not Solved 300.00
fhnw-binpack4-4 inf 250 108 0 0 0.0 0.0 Not Solved 300.00
fiball 6.75e-03 52 0 25 0 0.2 0.1 Not Solved 300.01
roi5alpha10n8 7.11e-01 101 776 0 0 5.2 3.5 Not Solved 300.07
map16715-04 1.10e+00 6 0 0 0 2.0 1.3 Not Solved 300.11
gen-ip002 8.43e-04 1 0 0 0 0.0 0.0 Not Solved 300.00
gfd-schedulen180f7d50m30k18 0.00e+00 64 64 64 64 11.8 27.9 Optimal 87.85
neos-5188808-nattai 7.91e-01 7 5 0 0 0.3 0.0 Not Solved 300.01
germanrr 7.28e-02 50 0 3 0 0.4 0.3 Not Solved 300.01
gen-ip054 4.54e-03 1 0 0 0 0.0 0.0 Not Solved 300.00
roll3000 7.48e-02 101 114 0 0 0.3 0.2 Not Solved 300.00
markshare2 1.00e+00 0 1 0 0 0.0 0.0 Not Solved 300.00
glass4 4.42e-01 51 53 0 0 0.0 0.0 Not Solved 300.00
neos-5195221-niemur 9.21e-01 42 0 0 0 0.5 0.0 Not Solved 300.01
glass-sc 1.55e-01 4 0 0 0 0.9 0.1 Not Solved 300.12
gmu-35-40 2.91e-04 3 1 0 0 0.0 0.0 Not Solved 300.00
s100 5.25e-02 29 0 24 0 6.7 25.3 Not Solved 300.39
markshare_4_0 1.00e+00 0 1 0 0 0.0 0.0 Not Solved 300.00
gmu-35-50 5.95e-04 3 0 0 0 0.0 0.0 Not Solved 300.00
neos5 1.67e-02 2 0 0 0 0.0 0.0 Not Solved 300.00
graph20-20-1rand 3.00e+00 8 0 0 0 0.1 0.1 Not Solved 300.00
s250r10 2.82e-03 22 0 0 0 5.7 4.7 Not Solved 300.34
mas74 2.85e-02 1 1 0 0 0.0 0.0 Not Solved 300.00
mas76 6.66e-03 1 0 0 0 0.0 0.0 Not Solved 300.00
neos-4300652-rahue 1.00e+00 9 0 0 0 1.2 0.3 Not Solved 300.04
neos-631710 8.62e-02 1 1 1 1 9.7 243.8 Not Solved 300.01
mc11 8.72e-01 580 0 7 0 0.9 0.8 Not Solved 300.01
satellites2-40 2.21e+00 0 0 0 0 4.5 208.3 Not Solved 300.00
mcsched 4.53e-02 3 0 0 0 0.1 0.0 Not Solved 300.00
mik-250-20-75-4 5.61e-02 25 47 28 0 0.0 0.0 Not Solved 300.00
milo-v12-6-r2-40-1 1.33e-01 599 82 4 0 2.3 9.2 Not Solved 300.15
neos-662469 8.34e-04 16 13 0 0 0.5 0.1 Not Solved 300.00
momentum1 1.92e-01 61 0 2 0 0.8 0.2 Not Solved 300.01
satellites2-60-fs 6.25e+00 7 7 7 7 4.2 231.9 Not Solved 300.01
mushroom-best 6.50e-01 - - - - 0.3 0.0 Not Solved 300.18
mzzv42z 2.05e-02 107 0 0 0 1.5 32.3 Not Solved 300.01
neos-1122047 0.00e+00 0 0 0 0 1.8 0.0 Optimal 8.30
neos-787933 6.98e-01 409 553 0 0 6.2 15.7 Not Solved 300.02
neos-827175 0.00e+00 0 19 0 0 0.1 0.1 Optimal 12.28
n2seq36q 3.83e-03 17 22 1 0 0.1 0.2 Not Solved 300.00
savsched1 9.40e-01 - - - - 0.0 0.0 Not Solved 300.20
neos-1171448 0.00e+00 132 0 0 0 0.6 0.8 Optimal 28.85
n5-3 5.40e-05 111 9 1 0 0.1 0.1 Optimal 180.62
n3div36 6.12e-02 3 14 11 0 0.5 0.2 Not Solved 300.01
neos-1445765 9.00e-05 1 0 0 0 0.0 0.0 Optimal 1.86
neos-1582420 9.30e-05 20 4 0 0 0.3 0.0 Optimal 134.30
neos-848589 8.44e-01 14 1009 0 0 7.4 0.9 Not Solved 300.08
neos859080 nan 1 0 1 0 0.0 0.0 Not Solved 0.78
neos-1171737 5.15e-03 31 0 0 0 0.2 0.1 Not Solved 300.00
sct2 2.04e-03 7 25 0 0 0.1 0.0 Not Solved 300.00
neos-1354092 7.34e-01 - - - - 0.0 0.0 Not Solved 300.00
seymour1 1.00e-04 24 0 0 0 0.3 0.2 Optimal 38.78
neos-860300 9.90e-05 10 0 0 0 0.1 0.0 Optimal 71.98
neos-1456979 1.27e-01 45 75 7 0 0.1 0.0 Not Solved 300.00
neos17 3.31e-01 14 16 0 0 0.0 0.0 Not Solved 300.00
neos-2075418-temuka nan 283 283 283 283 4.9 161.7 DUAL_UNBOUNDED 300.00
neos-2987310-joes 0.00e+00 - - - - 0.0 0.0 Optimal 300.00
neos-3004026-krka 0.00e+00 999 0 1001 0 0.1 0.0 Optimal 6.56
neos-2657525-crna 1.00e+00 6 5 3 0 0.0 0.0 Not Solved 300.00
seymour 2.73e-02 22 0 0 0 0.7 0.2 Not Solved 300.01
neos-873061 1.77e-01 40 3 2 0 4.9 89.0 Not Solved 300.03
neos8 4.12e-01 0 0 0 0 0.2 0.0 Optimal 0.88
neos-2746589-doon 1.28e-02 64 0 7 0 1.0 0.1 Not Solved 300.01
neos-3083819-nubu 5.90e-05 9 0 0 0 0.0 0.0 Optimal 30.48
neos-2978193-inde 1.21e-02 17 22 0 0 0.0 0.0 Not Solved 300.00
neos-3024952-loue 6.74e-01 414 0 4 0 0.9 0.3 Not Solved 300.00
sing326 5.62e-03 13 0 0 0 1.2 0.2 Not Solved 300.02
neos-3046615-murg 5.44e-01 0 9 0 0 0.0 0.0 Not Solved 300.00
neos-911970 5.88e-01 10 31 0 0 0.0 0.0 Not Solved 300.00
neos-3216931-puriri inf 20 0 0 0 0.3 3.1 Not Solved 300.01
neos-3381206-awhea 1.15e-01 4 445 0 0 0.0 0.1 Not Solved 300.00
neos-3402294-bobin 1.00e+00 18 0 0 0 0.2 1.2 Not Solved 302.22
sing44 9.79e-03 9 0 0 0 1.0 0.8 Not Solved 300.01
neos-933966 0.00e+00 4 0 0 0 1.2 1.0 Optimal 248.26
neos-3555904-turama 1.95e-01 5 0 1 0 1.5 0.0 Not Solved 300.31
neos-3402454-bohle inf 6 0 0 0 1.5 12.0 Not Solved 419.32
neos-3627168-kasai 1.50e-02 75 3 1 0 0.0 0.0 Not Solved 300.00
neos-3656078-kumeu 2.81e-01 862 74 118 0 6.3 151.5 Not Solved 300.01
snp-02-004-104 1.35e-02 46 8 0 0 0.3 0.0 Not Solved 300.01
neos-950242 6.25e-01 30 0 0 0 0.3 0.3 Not Solved 300.31
neos-3754480-nidda 3.18e+01 97 0 0 0 0.1 0.0 Not Solved 300.00
neos-957323 0.00e+00 7 0 0 0 0.4 0.9 Optimal 46.36
neos-4338804-snowy 2.56e-02 330 293 409 0 0.0 0.0 Not Solved 300.00
sorrell3 8.72e+00 72 0 0 0 7.7 0.2 Not Solved 300.65
physiciansched6-2 5.48e-02 412 115 36 0 0.6 0.9 Not Solved 300.00
wachplan 1.25e-01 6 0 0 0 0.1 0.0 Not Solved 300.00
neos-960392 0.00e+00 93 4 2 0 0.4 2.1 Optimal 50.12
ns1116954 0.00e+00 60 2 0 0 1.7 4.1 Optimal 26.35
netdiversion 0.00e+00 87 0 0 0 6.1 1.5 Optimal 121.59
net12 1.86e-01 207 6 2 0 4.9 6.4 Not Solved 300.04
ns1208400 0.00e+00 3 10 3 0 0.1 0.1 Optimal 162.15
nexp-150-20-8-5 6.95e-01 447 3 0 0 2.2 4.2 Not Solved 300.03
sp97ar 2.72e-02 7 10 1 0 0.3 0.1 Not Solved 300.01
sp150x300d 7.50e-02 56 1 19 0 0.0 0.0 Not Solved 300.00
ns1952667 0.00e+00 1 0 18 0 0.1 0.0 Optimal 99.84
ns1644855 1.89e-01 15 0 0 0 1.7 4.0 Not Solved 300.03
ns1760995 1.58e+00 - - - - 0.0 0.0 Not Solved 300.00
ns1830653 2.21e-01 62 17 1 0 0.5 0.3 Not Solved 300.01
nw04 3.30e-05 2 0 0 0 0.1 0.1 Optimal 39.89
sp98ar 1.28e-02 24 20 3 0 0.9 0.2 Not Solved 300.01
splice1k1 3.18e+00 1 0 0 0 2.2 0.1 Not Solved 328.52
nu25-pr12 4.06e-03 29 0 7 0 0.1 0.0 Not Solved 300.00
nursesched-medium-hint03 9.58e-01 546 0 184 0 23.7 72.7 Not Solved 300.03
nursesched-sprint02 1.72e-02 75 0 17 0 0.8 0.2 Not Solved 300.01
opm2-z10-s4 4.86e-01 3 0 0 0 13.1 2.2 Not Solved 300.98
square41 4.38e-01 1 0 18 0 11.2 28.1 Not Solved 300.39
square47 4.85e-01 105 105 105 105 9.4 141.7 Not Solved 302.03
p200x1188c 2.37e-01 13 0 1 0 0.1 0.0 Not Solved 300.00
peg-solitaire-a3 inf 10 5 6 0 0.4 0.1 Not Solved 300.00
pg5_34 6.95e-03 20 787 0 0 0.1 0.2 Not Solved 300.00
piperout-08 0.00e+00 1157 0 2 0 0.2 0.2 Optimal 10.75
piperout-27 0.00e+00 1917 1 3 0 0.7 0.3 Optimal 8.22
pg 2.72e-01 91 90 0 0 0.1 0.0 Not Solved 300.00
supportcase10 5.74e-01 2 0 0 0 11.6 6.3 Not Solved 300.01
supportcase12 7.33e-03 29 164 0 0 0.2 3.0 Not Solved 300.01
physiciansched3-3 1.01e-01 30 30 30 30 4.0 228.0 Not Solved 300.00
radiationm18-12-05 2.74e-04 131 4 51 0 0.1 0.1 Not Solved 300.00
pk1 1.00e-04 1 0 0 0 0.0 0.0 Optimal 224.32
proteindesign121hz512p9 3.39e-02 86 14 56 0 0.7 0.1 Not Solved 300.02
qap10 0.00e+00 2 0 579 0 1.3 42.4 Optimal 134.76
supportcase18 5.63e-02 39 69 10 0 0.0 0.1 Not Solved 300.00
supportcase19 inf - - - - 0.0 0.0 Not Solved 300.01
proteindesign122trx11p8 1.45e-02 146 11 58 0 1.4 0.5 Not Solved 300.02
unitcal_7 1.82e-02 52 0 0 0 0.7 0.1 Not Solved 300.01
supportcase22 inf 0 0 0 0 2.4 288.9 Not Solved 300.00
supportcase26 9.66e-02 0 3 0 0 0.0 0.0 Not Solved 300.00
supportcase33 1.32e-01 85 36 1 0 0.7 0.4 Not Solved 300.01
supportcase7 9.90e-05 1062 371 0 0 2.1 1.7 Optimal 30.48
supportcase40 6.84e-03 87 0 0 0 0.3 0.2 Not Solved 300.01
swath1 1.00e-04 8 13 1 0 0.0 0.0 Optimal 60.04
supportcase6 9.50e-05 11 0 23 0 3.0 7.0 Optimal 70.22
supportcase42 8.18e-02 16 0 0 0 0.7 0.0 Not Solved 300.01
swath3 7.42e-02 23 24 1 0 0.1 0.0 Not Solved 300.00
tbfp-network 1.03e-01 6 0 59 0 5.4 126.1 Not Solved 300.10
thor50dday 8.72e-01 333 0 0 0 10.6 33.2 Not Solved 300.06
tr12-30 5.31e-02 987 101 0 0 0.2 0.1 Not Solved 300.00
timtab1 2.75e-01 182 172 3 0 0.0 0.0 Not Solved 300.00
uccase12 2.90e-05 32 7 0 0 2.8 0.1 Optimal 26.95
traininstance2 1.00e+00 17 14 0 0 0.1 0.0 Not Solved 300.00
traininstance6 8.06e-01 0 4 3 0 0.0 0.0 Not Solved 300.00
trento1 1.17e-04 2 0 0 0 0.4 0.1 Not Solved 300.01
triptim1 0.00e+00 2 0 1 0 6.8 0.1 Optimal 246.65
mzzv11 3.14e-02 43 0 0 0 0.7 0.8 Not Solved 300.01
There was a problem hiding this comment.
Instead of adding all these time checks all over the code, I think we should take a look at the worst offenders and only add time checks as necessary.
chris-maes
left a comment
chris-maes
left a comment
There was a problem hiding this comment.
I'm worried this goes overboard in checking the time limit. When we add to many checks it becomes difficult to reason about the code.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cpp/src/dual_simplex/types.hpp (1)
23-25: Macros defined inside namespace have no actual scoping.Both
CONCURRENT_HALT_RETURNandTIME_LIMIT_RETURNare#definemacros, so they leak outside the namespace. This is pre-existing forCONCURRENT_HALT_RETURN, but as you're adding a new constant, consider usingconstexpr i_t(orconstexpr int) instead to get proper namespace scoping. This would also give you type safety.♻️ Suggested change
-// We return this constant to signal that a concurrent halt has occurred -#define CONCURRENT_HALT_RETURN -2 -// We return this constant to signal that a time limit has occurred -#define TIME_LIMIT_RETURN -3 +// We return this constant to signal that a concurrent halt has occurred +constexpr int CONCURRENT_HALT_RETURN = -2; +// We return this constant to signal that a time limit has occurred +constexpr int TIME_LIMIT_RETURN = -3;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cpp/src/dual_simplex/types.hpp` around lines 23 - 25, Replace the two preprocessor macros CONCURRENT_HALT_RETURN and TIME_LIMIT_RETURN with properly namespaced typed constants (e.g., inside the dual_simplex namespace declare constexpr i_t CONCURRENT_HALT_RETURN = -2; and constexpr i_t TIME_LIMIT_RETURN = -3; or use constexpr int if i_t isn't available), remove the corresponding `#define` lines, and update any call sites that expect those macro names to use the new constants so they benefit from scope and type safety.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cpp/src/branch_and_bound/branch_and_bound.cpp`:
- Around line 2306-2310: The if-block calls set_final_solution(...) redundantly
because stop_for_time_limit() already invokes set_final_solution(solution,
root_objective_) when it returns true; remove the duplicate call inside the
block at the location checking stop_for_time_limit() || scratch_status ==
lp_status_t::TIME_LIMIT so that when stop_for_time_limit() is true we only set
solver_status_ = mip_status_t::TIME_LIMIT and return, but still retain a call to
set_final_solution(...) when the condition is true due solely to scratch_status
== lp_status_t::TIME_LIMIT (i.e., only perform set_final_solution when
scratch_status == lp_status_t::TIME_LIMIT, not when stop_for_time_limit()
returned true).
---
Nitpick comments:
In `@cpp/src/dual_simplex/types.hpp`:
- Around line 23-25: Replace the two preprocessor macros CONCURRENT_HALT_RETURN
and TIME_LIMIT_RETURN with properly namespaced typed constants (e.g., inside the
dual_simplex namespace declare constexpr i_t CONCURRENT_HALT_RETURN = -2; and
constexpr i_t TIME_LIMIT_RETURN = -3; or use constexpr int if i_t isn't
available), remove the corresponding `#define` lines, and update any call sites
that expect those macro names to use the new constants so they benefit from
scope and type safety.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cpp/src/branch_and_bound/branch_and_bound.hpp (1)
109-110:stop_for_time_limitshould be private.This method is only called internally by
solve()and related private methods. Exposing it in the public API unnecessarily widens the class surface.Suggested fix: move to private section
Move the declaration from the current public block (around line 109) into the private section alongside
set_final_solution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cpp/src/branch_and_bound/branch_and_bound.hpp` around lines 109 - 110, The declaration of stop_for_time_limit(mip_solution_t<i_t, f_t>& solution) is public but only used internally by solve() and other private helpers; move its declaration into the private section of the BranchAndBound class next to set_final_solution so it is not exposed in the public API. Locate stop_for_time_limit in branch_and_bound.hpp and cut its prototype from the public block and paste it under the existing private members (near set_final_solution) so only solve() and private methods can call it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cpp/src/branch_and_bound/branch_and_bound.hpp`:
- Around line 109-110: The declaration of
stop_for_time_limit(mip_solution_t<i_t, f_t>& solution) is public but only used
internally by solve() and other private helpers; move its declaration into the
private section of the BranchAndBound class next to set_final_solution so it is
not exposed in the public API. Locate stop_for_time_limit in
branch_and_bound.hpp and cut its prototype from the public block and paste it
under the existing private members (near set_final_solution) so only solve() and
private methods can call it.
| rank = right_looking_lu(A, settings, medium_tol, basic_list, q, L, U, pinv); | ||
| rank = right_looking_lu(A, settings, medium_tol, basic_list, q, L, U, pinv, start_time); | ||
| if (rank < 0) { | ||
| if (settings.concurrent_halt != nullptr && *settings.concurrent_halt == 1) { |
There was a problem hiding this comment.
We check for concurrent halt below. So not sure this is necessary.
If you think it should be kept, please remove the check below.
There was a problem hiding this comment.
This is when right_looking_lu returns some non-success code but concurrent LP has found. So this is to basically return the correct return code and logs.
| settings.log.printf("Concurrent halt\n"); | ||
| return CONCURRENT_HALT_RETURN; | ||
| } | ||
| if (Srank < 0) { return Srank; } |
There was a problem hiding this comment.
Probably better to handle the TIME_LIMIT case here explicitly.
There was a problem hiding this comment.
This just propagates the error to the call stack. The callstack functions handle different return codes correctly(crossover.cpp and primal.coo). I can put a comment if you want?
| std::vector<i_t>& deficient, | ||
| std::vector<i_t>& slacks_need); | ||
| std::vector<i_t>& slacks_need, | ||
| f_t start_time); |
There was a problem hiding this comment.
Can you move start_time up here as well before merging?
| std::vector<i_t>& nonbasic_list, | ||
| std::vector<variable_status_t>& vstatus) | ||
| std::vector<variable_status_t>& vstatus, | ||
| f_t start_time) |
There was a problem hiding this comment.
Can you move start_time to be with the input arguments here as well
| std::vector<i_t>& nonbasic_list, | ||
| std::vector<variable_status_t>& vstatus); | ||
| std::vector<variable_status_t>& vstatus, | ||
| f_t start_time); |
There was a problem hiding this comment.
Can you move start_time to be with the input arguments here as well?
chris-maes
left a comment
chris-maes
left a comment
There was a problem hiding this comment.
LGTM. Thanks for adding all these timer checks.
Would you mind moving start_time to appear with the input arguments of the functions I commented on, before merging?
…zero slack values left in the previous solution
|
/merge |
3 participants
This PR adds more timer checks in cuts and in strong branching.
Summary by CodeRabbit
Bug Fixes
Chores