Mip race

[jajhall]

Some residual code from MIP race experience.

HighsTerminator struct allows termination of the MIP solver to be communicated more readily - and across multiple threads

MipSolutionSource now ordered alphabetically by key letters

[codecov]

Codecov Report

❌ Patch coverage is 70.87379% with 60 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.69%. Comparing base (cfd986d) to head (6b4a6ee).
⚠️ Report is 89 commits behind head on latest.

Files with missing lines	Patch %	Lines
highs/mip/HighsMipSolverData.cpp	67.30%	34 Missing ⚠️
highs/mip/HighsMipSolver.cpp	70.90%	16 Missing ⚠️
highs/lp_data/HighsModelUtils.cpp	0.00%	4 Missing ⚠️
highs/mip/HighsPrimalHeuristics.cpp	55.55%	4 Missing ⚠️
highs/mip/HighsMipSolver.h	50.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           latest    #2474      +/-   ##
==========================================
- Coverage   79.73%   79.69%   -0.04%     
==========================================
  Files         346      346              
  Lines       85976    86096     +120     
==========================================
+ Hits        68553    68616      +63     
- Misses      17423    17480      +57     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jajhall]

Now that solution of sub-MIPs can be terminated, the MIP solver instances terminated in a race now take only a little longer than the winner.

Hence we have a 7X speedup on fiball

Further experiments will follow once I've cleaned up the dev printf statements

[mathgeekcoder]

Nice trick with the start/finish checks - but I believe there's still a chance where this could fail in a race condition... especially if some "smart" compiler optimizes away the final this->start_write_incumbent == start_write_incumbent check.

Perhaps consider using std::atomic<HighsInt> for your start/finish variables? The atomic class is lock free and very fast on most systems.

struct MipRaceIncumbent {
  std::atomic<HighsInt> start_write_incumbent = kMipRaceNoSolution;
  std::atomic<HighsInt> finish_write_incumbent = kMipRaceNoSolution;
  double objective = -kHighsInf;
  std::vector<double> solution;
  void clear();
  void initialise(const HighsInt num_col);
  void update(const double objective, const std::vector<double>& solution);
  HighsInt read(const HighsInt last_incumbent_read, double& objective_,
                std::vector<double>& solution_) const;

  MipRaceIncumbent() = default;

  MipRaceIncumbent(const MipRaceIncumbent& copy) {
    start_write_incumbent = copy.start_write_incumbent.load();
    finish_write_incumbent = copy.finish_write_incumbent.load();
    objective = copy.objective;
    solution = copy.solution;
  }

  MipRaceIncumbent(MipRaceIncumbent&& moving) {
    start_write_incumbent = moving.start_write_incumbent.load();
    finish_write_incumbent = moving.finish_write_incumbent.load();
    objective = moving.objective;
    solution = std::move(moving.solution);
  }
};

[mathgeekcoder]

That said, I'm guessing you're trying to avoid heavier std::mutex for thread synchronization. As a lighter alternative, you can also use std::atomic_flag for a SpinLock implementation. For example:

class Spinlock {
private:
  std::atomic_flag lock_flag = ATOMIC_FLAG_INIT;

public:
  void lock() {
    while (lock_flag.test_and_set(std::memory_order_acquire)) {
      // Busy-wait (spin) until the lock is released
    }
  }
  
  bool try_lock() noexcept {
    return lock_flag.test_and_set(std::memory_order_acquire);
  }
  void unlock() { lock_flag.clear(std::memory_order_release); }
};

Note: There is likely better SpinLock code available.

[mathgeekcoder]

Oh! I forgot that we already have HighsSpinMutex. So ignore my SpinLock code above.

[jajhall]

I tried implementing the new struct MipRaceIncumbent, but got the following compiler errors

/home/jajhall/HiGHS/highs/mip/HighsMipSolver.h:38:25: error: field ‘start_write_incumbent’ has incomplete type ‘std::atomic’
38 | std::atomic start_write_incumbent = kMipRaceNoSolution;
| ^~~~~~~~~~~~~~~~~~~~~

[mathgeekcoder]

Not entirely sure. Did you #include <atomic>?

[mathgeekcoder]

Ah, sorry. That's a linux vs windows issue. The following should work on both.

std::atomic<HighsInt> start_write_incumbent {kMipRaceNoSolution}; 
std::atomic<HighsInt> finish_write_incumbent {kMipRaceNoSolution};

[jajhall]

Not entirely sure. Did you #include <atomic>?

Yes, but doesn't fix the error

Ah, sorry. That's a linux vs windows issue. The following should work on both.

Works on Linux! :-)

[mathgeekcoder]

BTW: AFAIK this inner for loop will execute only on one thread, so it might be a bit pointless if it loops more than once (i.e., that means a previous HighsMipSolver::run has already finished).

That said, the default grain size = 1, so it's unlikely to ever occur. You could probably get rid of the inner for loop and just take instance = start instead.

Alternatively, you could try to use TaskGroup directly and spawn each task. It would be more effort, but you might've been able to use the TaskGroup::cancel function to interrupt the other threads.

[mathgeekcoder]

The newSolution method is missing the & for the double objective. Without this, the solution sharing across threads isn't updating as frequently as it could.

bool newSolution(const HighsInt instance, double& objective,
                   std::vector<double>& solution);

[jajhall]

That's an important omission! It meant that the objective for the solution read from another thread wasn't being passed back, so the solution from the other thread never replaced the incumbent. Hence, sharing solutions was the same as a pure race.

Making the correction, the performance on fiball when sharing solutions is slowed greatly, as none of the threads runs like the "lucky" random_seed=1 case. This is what I'd expected to see. However, for problems where there isn't an extremely lucky random_seed value, maybe this will open the door to improved performance!

Naturally I'll experiment

[mathgeekcoder]

I noticed similar behaviour in my experiments. Sharing solutions can help on other instances, but not really on fiball. That said, when I performed the presolve once and shared that across the other threads, fiball was fast again.

BTW: I had to make many changes to ensure presolve is only performed once. Do you know a better way?

[mathgeekcoder]

BTW: After more experiments with fiball, sharing the presolve doesn't necessarily solve it fast. More investigation needed!

[jajhall]

Now I've thought a little, it's easy internally to perform presolve only once, and then race the solution of the presolved MIP

[mathgeekcoder]

Minor fix: \% should be %%

i.e., " Solver %d has best objective %15.8g, gap %6.2f\% (time "

[jajhall]

Corrected

[mathgeekcoder]

I believe there is a bug with the objective value when mipsolver.model_->offset_ != 0. This can prevent improved solutions from being accepted by other threads.

Details:
addIncumbent checks instance_solution_objective_value < upper_bound before updating, but upper_bound excludes the mipsolver.model_->offset_ value and instance_solution_objective_value includes it.

Potential fix:

    // exclude offset from objective value
    instance_solution_objective_value -= mipsolver.model_->offset_;

    addIncumbent(reduced_instance_solution, instance_solution_objective_value,
                 kSolutionSourceHighsSolution);

[jajhall]

Thanks: the correction depends on whether the problem is a maximization or minimization

[mathgeekcoder]

Good to know. I was thinking the sign for offset_ might've already accounted for min/max.

[jajhall]

Fixed

[jajhall]

Can now perform presolve before the MIP race so that all participants are solving the same problem.

Controlled by option mip_race_single_presolve which is true by default.

[jajhall]

The performance gain from the MIP race didn't justify introducing a non-deterministic solver into HiGHS, or the investment of time required to fix the segfaults and incorrect deductions of infeasibility.

All the code relating to the MIP race has been deleted so that the one useful outcome - the HighsTerminator can be merged into latest

[jajhall]

The performance gain from the MIP race didn't justify introducing a non-deterministic solver into HiGHS, or the investment of time required to fix the segfaults and incorrect deductions of infeasibility.

All the code relating to the MIP race has been deleted so that the one useful outcome - the HighsTerminator can be merged into latest