-
Notifications
You must be signed in to change notification settings - Fork 115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wrong result for ILP, regression in the last 1 or 2 months #668
Comments
Could you use |
That is not that easy as I use a mirror that just not contains all that stuff and can't use the repos as is as I need it CMake based. If I have more hints where the issue might be, I can add that here. I have a bunch of ILPs with results 'verified' with a second solver like cplex that I use as regression tests, is there some way to add such stuff to the auto tests? https://github.com/christoph-cullmann/ilp At least all x months they find regressions in cbc and Co. |
I have many versions of master. The one I am working on most now lacks many recent updates and gets correct answer. A vanilla up to date master does get the answer wrong - so I can slowly find problem. At a first glance it looks like a bad cut generator. If I add -cgraph clq to go back to old type cliques it solves correctly. As to auto tests. I have a directory ~/cullmann with files and a file testset in that directory and then |
Problem is CoinStaticConflictGraph - may take some time to work out what it does/did. |
Thanks as lot for investigating this! If it is of any help, the other test that has different results for me with the current version is https://github.com/christoph-cullmann/ilp/blob/main/value_28841479959.lp That is now just infeasible:
|
Many models on standard tests get wrong answer too! The problem was introduced by using std::vector more. You can get use of uninitialized values in CoinAdjacencyVector (that is easily fixed). However the problem here is using size() as a test for whether a vector is empty in CoinStaticConflictGraph. Maybe someone who know code better than I do could fix - otherwise it will take a bit of time. |
Hope I have fixed it in master. |
I think the std::vector changes were part of PR240 on CoinUtils. There may be more surprises there. |
Thanks, at least my initial regression tests work for all things that did work in the old release I had. |
I would recommend rolling back the changes to vector.
When Samuel was developing the conflict graph infrastructure, we did some
benchmarking on code that used it versus code with pointers and arrays and
performance degradation with vectors was noticeable.
…On Fri, Aug 30, 2024, 03:20 John Forrest ***@***.***> wrote:
Many models on standard tests get wrong answer too!
The problem was introduced by using std::vector more. You can get use of
uninitialized values in CoinAdjacencyVector (that is easily fixed). However
the problem here is using size() as a test for whether a vector is empty in
CoinStaticConflictGraph. Maybe someone who know code better than I do could
fix - otherwise it will take a bit of time.
—
Reply to this email directly, view it on GitHub
<#668 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB4VZOXWWMJI42NSBGHVJ4TZUBBOLAVCNFSM6AAAAABNGFJHJWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRQG43DIOJRGU>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
Can you tell me where the Cmakelist.txt file is hiding, I have looked everywhere for it? |
See the attached ILP
value_3810812.lp.gz
I updated my local CBC to current master (including osi,coinutils,...), now cbc computes a different result (mismatching the cplex one)
The text was updated successfully, but these errors were encountered: