Conversation
This reverts commit 6f62e0e.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## latest #2674 +/- ##
==========================================
+ Coverage 81.20% 81.37% +0.16%
==========================================
Files 349 349
Lines 85465 85957 +492
==========================================
+ Hits 69406 69950 +544
+ Misses 16059 16007 -52 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fwesselm
left a comment
There was a problem hiding this comment.
Looks good, @Opt-Mucca. I will try to qualify as soon as possible.
|
It would be better to maintain the use of @Opt-Mucca Please make this modification so we can merge this PR |
|
@jajhall Can do. |
|
I'm going to close this PR for now. The fixed ordering of rows per column seems to always lead to negative performance and I don't think the trade-off for stability is worth it. |
This PR sorts rows via their
fractionalActivityinHighsPathSeparatorwhen considering which row to add to the aggregation.High level overview: The separator tries to find a continuous column to aggregate out, and to do so finds an additional row that also contains that continuous column. Which row to use is non-obvious.
The old method:
The new method:
Pros:
array[i-1]is checked beforearray[i]in all cases aside from starting the scan ati.Cons:
This should only be merged after doing a performance run.