Normal equations constructed in parallel

[filikat]

• The structure (once per IPM solve) and values (once per IPM iteration) of the normal equations are built in parallel. This is only done if matrix A is sufficiently large (1e4 rows or columns) and dense (10 nz per col or 30 nz per row). Building values is parallelised easily, as the structure is already computed and different threads write to different entries. Building the structure is more complicated, as the total number of nonzeros is not known. Each thread uses a local buffer and the final structure is assemble serially.
• Running of the multiple orderings in parallel is now performed regardless of the parallel option.
• The Highs random seed is now passed to Metis (with an offset, so that current behaviour is not perturbed, and it's easier to compare results with previous versions). This is the only place where the random seed has an effect on HiPO.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 89 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.92%. Comparing base (2eb3245) to head (5d2e9ed).
⚠️ Report is 1 commits behind head on latest.

Files with missing lines	Patch %	Lines
highs/ipm/hipo/ipm/KktMatrix.cpp	0.00%	65 Missing ⚠️
highs/ipm/hipo/ipm/FactorHighsSolver.cpp	0.00%	20 Missing ⚠️
highs/ipm/hipo/ipm/Solver.cpp	0.00%	2 Missing ⚠️
highs/ipm/hipo/factorhighs/FactorHighs.cpp	0.00%	1 Missing ⚠️
highs/ipm/hipo/factorhighs/FactorHighs_c_api.cpp	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           latest    #3187      +/-   ##
==========================================
- Coverage   72.95%   72.92%   -0.03%     
==========================================
  Files         436      436              
  Lines      105985   106033      +48     
  Branches    17066    17071       +5     
==========================================
+ Hits        77323    77327       +4     
- Misses      28386    28430      +44     
  Partials      276      276              

☔ View full report in Codecov by Harness.
📢 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]

I'm a bit shocked by the comment "vector is not thread-safe" - why? - as I'm sure it's used elsewhere!

Otherwise, apart from the random_seed offset of 42 (why?) this looks good to me

[filikat]

I'm a bit shocked by the comment "vector is not thread-safe" - why? - as I'm sure it's used elsewhere!

std::vector<bool> is not thread-safe (from https://en.cppreference.com/cpp/container/vector_bool: "Does not guarantee that different elements in the same container can be modified concurrently by different threads").
std::vector<T> for any other T is thread safe.

Otherwise, apart from the random_seed offset of 42 (why?) this looks good to me

It's just the random seed that I was passing to Metis up to now (https://en.wikipedia.org/wiki/Phrases_from_The_Hitchhiker's_Guide_to_the_Galaxy#The_Answer_to_the_Ultimate_Question_of_Life,_the_Universe,_and_Everything_is_42).
I can remove the offset, but it makes it a bit harder to compare future results with past runs.

[jajhall]

I'm a bit shocked by the comment "vector is not thread-safe" - why? - as I'm sure it's used elsewhere!

std::vector<bool> is not thread-safe (from https://en.cppreference.com/cpp/container/vector_bool: "Does not guarantee that different elements in the same container can be modified concurrently by different threads"). std::vector<T> for any other T is thread safe.

So, do we get rid of other occurrences of std::vector<bool> in HiGHS? Or only the ones where modification by different threads is possible - and I'm unaware of any such cases.

Otherwise, apart from the random_seed offset of 42 (why?) this looks good to me

It's just the random seed that I was passing to Metis up to now (https://en.wikipedia.org/wiki/Phrases_from_The_Hitchhiker's_Guide_to_the_Galaxy#The_Answer_to_the_Ultimate_Question_of_Life,_the_Universe,_and_Everything_is_42). I can remove the offset, but it makes it a bit harder to compare future results with past runs.

No problem, I know the reference, and it adds a human touch to the code

[filikat]

So, do we get rid of other occurrences of std::vector<bool> in HiGHS? Or only the ones where modification by different threads is possible - and I'm unaware of any such cases.

This has been an issue for me (thread sanitizer was failing and it took some time to figure out that std::vector<bool> was the problem) and for Ivet (#3025, same issue). I think it would be good to eventually get rid of all instances and replace them with std::vector<char> or std::vector<uint8_t>, but there is no rush to do this. As you mentioned, I don't think that std::vector<bool> is used concurrently anywhere at the moment.

[jajhall]

So, do we get rid of other occurrences of std::vector<bool> in HiGHS? Or only the ones where modification by different threads is possible - and I'm unaware of any such cases.

This has been an issue for me (thread sanitizer was failing and it took some time to figure out that std::vector<bool> was the problem) and for Ivet (#3025, same issue). I think it would be good to eventually get rid of all instances and replace them with std::vector<char> or std::vector<uint8_t>, but there is no rush to do this. As you mentioned, I don't think that std::vector<bool> is used concurrently anywhere at the moment.

I've just done a search, and there are about 20 std::vector in HiGHS, including the following

/home/jajhall/HiGHS/highs/ipm/hipo/factorhighs
Analyse.cpp:213: std::vector is_sn(n_, false);
/home/jajhall/HiGHS/highs/ipm/hipo/ipm
KktMatrix.cpp:137: std::vector is_nz(m, false);
Model.cpp:47: std::vector mark(m_, false);
UpLookingSolver.cpp:95: std::vector mark(n_, false);

I'll create an issue to remind ourselves to clear them all!

[Opt-Mucca]

@filikat I've been using uint8_t for parallel MIP (I find it nicer than char)