Refactor the HiPO build system to support cross compilation

[odow]

x-ref #2542

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (hipo-dev@3d19148). Learn more about missing BASE report.

Additional details and impacted files

@@             Coverage Diff             @@
##             hipo-dev    #2550   +/-   ##
===========================================
  Coverage            ?   81.05%           
===========================================
  Files               ?      347           
  Lines               ?    86974           
  Branches            ?        0           
===========================================
  Hits                ?    70500           
  Misses              ?    16474           
  Partials            ?        0           

☔ 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.

[odow]

@amontoison: I see that you forked METIS for Julia. What's the status of upstream? Should I just grab your binaries instead?

[amontoison]

@odow The original website is not working anymore but it seems that they did the new web page now:
https://karypis.github.io/glaros/software/metis/overview.html

I suggest to grap my binaries because it contains a few patches, particularly for Windows.

[odow]

@filikat do you know about these segfaults on Windows?

[filikat]

@odow Ivet told me about them, but I don't know why they happen and I don't have a machine with Windows.
I had a look at the log files of the checks that failed. It looks like the debug build works, but the release build gives segfault.
The debug build with tests on fails because one of the checks in TestHipo.cpp fails. This is the test that checks if running HiPO twice with the same problem produces the exact same solution. Could there be sometihng non deterministic when running on Windows? I am not sure of exactly what BLAS library gets linked on Windows and whether it is fully deterministic.

[odow]

So this happens in Julia because we force solver = ipm in one of the tests. I think we should use HiPO only if we force solver = hipo.

[odow]

The related issue is that we have re-interpreted solver = ipm to previously mean IPX and now mean IPX | HiPO, and there is a new solver = "ipx" option for IPX.

HiGHS/highs/lp_data/HighsOptions.h

Lines 260 to 262 in 364c83a

	const string kSimplexString = "simplex";
	const string kIpmString = "ipm";
	const string kPdlpString = "pdlp";

This is potentially a breaking change for folks like GenX which force the solver to ipm. People with the default or choose should expect the algorithm to change between HIGHS versions. But not if they are explicitly choosing the algorithm.

[filikat]

The related issue is that we have re-interpreted solver = ipm to previously mean IPX and now mean IPX | HiPO, and there is a new solver = "ipx" option for IPX.

HiGHS/highs/lp_data/HighsOptions.h

Lines 260 to 262 in 364c83a

	const string kSimplexString = "simplex";
	const string kIpmString = "ipm";
	const string kPdlpString = "pdlp";

This is potentially a breaking change for folks like GenX which force the solver to ipm. People with the default or choose should expect the algorithm to change between HIGHS versions. But not if they are explicitly choosing the algorithm.

I agree with you, we should maintain compatibility for people who were selecting "ipm" thinking that they got IPX.
I thought that it was already the case honestly.

[jajhall]

I made this change deliberately as it aligns with what will happen in future. The ipm string can no longer be interpreted as a specific implementation.

[jajhall]

@filikat here's a bug in HiPO. It doesn't detect primal unboundedness:

Thanks, I will have a look. The detection of infeasibility/unboundedness is very simplistic for now and it needs some tuning.

Don't worry about this for now. The priority is performance for feasible LPs

[filikat]

But of course, when I hit it with lldb it now works...

@odow Could you try to run the same problem, setting output_flag to true for the ipm in the analytic centre computation and log_dev_level to 1. Maybe we can see at what point the ipm fails.

[filikat]

The debug build with tests on fails because one of the checks in TestHipo.cpp fails. This is the test that checks if running HiPO twice with the same problem produces the exact same solution. Could there be sometihng non deterministic when running on Windows?

I don't think non-determinism is the problem. By the looks of it, you set openblas_set_num_threads(1) and you fix the METIS_OPTION_SEED.

Isn't it just a case that you've solved the problem, then you're warm-starting from a different solution? It's not the same as solving two models independently.

@odow I see. I will change the test so that it uses two separate Highs objects, just to be sure.

[jajhall]

@odow Please revert the changes of

const string kIpmString = "ipm";
const string kHipoString = "hipo";
const string kIpxString = "ipx";

to

const string kIpmString = "interior-point";
const string kHipoString = "hipo";
const string kIpxString = "ipm";

since IPM should not be synonymous with IPX, and the corresponding changes in TestSpecialLps.

[jajhall]

The related issue is that we have re-interpreted solver = ipm to previously mean IPX and now mean IPX | HiPO, and there is a new solver = "ipx" option for IPX.

HiGHS/highs/lp_data/HighsOptions.h

Lines 260 to 262 in 364c83a

	const string kSimplexString = "simplex";
	const string kIpmString = "ipm";
	const string kPdlpString = "pdlp";

This is potentially a breaking change for folks like GenX which force the solver to ipm. People with the default or choose should expect the algorithm to change between HIGHS versions. But not if they are explicitly choosing the algorithm.

@odow How can solver=ipm mean anything other than "use interior point to solve the LP"?

With my changes that's still happening - except that HiPO (when available) is being used by default. Anyone who is setting solver=ipm rather than the default value solver=choose (which implies simplex) is, most likely, solving big enough LPs that they want IPM rather than simplex. Hence it seems reasonable that the better IPM solver for large LPs - ie HiPO - is used by default.

All that's changed is that solver=ipm is now a "choice" setting, with HiGHS choosing HiPO.

In due course HiGHS will choose between IPX and HiPO, using the former for small LPs, or on the basis of other characteristics.

Similarly, in due course, with solver=choose HiGHS will choose between simplex, IPX and HiPO

If people wanting IPM are finding that HiPO is slower or less reliable than IPX, it's not difficult for them to change a single parameter to force the use of IPX.

[odow]

This is a draft WIP PR. My goal is just to get the tests passing.

Re ipm: This just shows the difficulty of naming things with backward compatibility. Fully agree that ipm should mean "solve with the interior point method of your choice" but people currently think means "use IPX". (I don't think people know that the "ipm" solver in HiGHS is called "ipx".)

Until HiPO is mature, I think we should be making HiPO explicitly opt-in.

IMO: the priority should be: fixing Windows segfaults, fixing failing unit tests, making it so we can actually compile and distribute HiPO. Once it's in latest and people are using it, then we can improve performance.

[jajhall]

This is a draft WIP PR. My goal is just to get the tests passing.

Agreed, which is why I was surprised at you making changes to solver code.

Re ipm: This just shows the difficulty of naming things with backward compatibility. Fully agree that ipm should mean "solve with the interior point method of your choice" but people currently think means "use IPX". (I don't think people know that the "ipm" solver in HiGHS is called "ipx".)

So solver=ipm was correct. The error would have been to have the option solver=ipx for IPM when there was only one

If people don't know that the "ipm" solver in HiGHS is called "ipx", I fail to see the problem with switching the default IPM solver

Until HiPO is mature, I think we should be making HiPO explicitly opt-in.

I note your opinion. However, by making HiPO default when solver=ipm (which few users will do) will expose it to a lot more models, and most users will get a meaningful performance enhancement. Those who experience slow-down or failure can contribute their instances and help @filikat improve the performance and robustness of HiPO. This is already happening.

IMO: the priority should be: fixing Windows segfaults, fixing failing unit tests, making it so we can actually compile and distribute HiPO. Once it's in latest and people are using it, then we can improve performance.

I note your opinion, but @filikat has only so much to contribute in this area. His priority is performance and robustness. At this stage, infeasibility detection is not important.

[odow]

Trying here: #2572

[odow]

If we're patching METIS, we could just carry the patch so that glib doesn't need to be linked explicitly? There really should be no need for HiGHS to know about gklib

[filikat]

@galabovaa it seems like hipo-tt-dev-gk doesn't include the latest changes from hipo-dev. Maybe you made the branch starting from hipo instead?

[odow]

What if I just use your patched version of METIS here? (Oh, because of the gklib stuff)

[filikat]

If we're patching METIS, we could just carry the patch so that glib doesn't need to be linked explicitly? There really should be no need for HiGHS to know about gklib

I think that's one of the patches that Ivet applied to her version of Metis (https://github.com/galabovaa/METIS/tree/510-w)

[galabovaa]

@filikat

@galabovaa it seems like hipo-tt-dev-gk doesn't include the latest changes from hipo-dev. Maybe you made the branch starting from hipo instead?

I did start from hipo, I thought I merged hipo-dev but must have forgotten to pull. I will merge it tomorrow when we meet

@odow

If we're patching METIS, we could just carry the patch so that glib doesn't need to be linked explicitly? There really should be no need for HiGHS to know about gklib

The GKlib path is only optional as we discussed, with the patched up metis or vcpkg you just don't specify it. Sure, I can add the julia workflows from this PR too

[filikat]

I did start from hipo, I thought I merged hipo-dev but must have forgotten to pull. I will merge it tomorrow when we meet

Looks like you merged hipo-dev into hipo-tt-dev but not hipo-tt-dev-gk

[odow]

I'm seeing lots of green! Looks like Ivet's patched METIS did the trick.

[odow]

Why do we have hipo and hipo-dev?

[filikat]

Why do we have hipo and hipo-dev?

Some people are already using the code in hipo. I use hipo-dev for stuff that I'm working on, before moving it to hipo when it works. It's just for now, once hipo gets merged in latest, hipo will become the working branch for me.

[odow]

I've rebased onto hipo-dev. Fingers crossed I didn't make a mistake...

[odow]

I've rebased this onto the latest hipo-dev.

What's the status of this?

If we don't like detecting openblas/accelerate from the string, another option for setting HIPO_USES_OPENBLAS|ACCELERATE is for them to be options that are explicitly set by user when compiling?

[filikat]

@odow Ivet is making progress inhipo-tt. Once she finishes, we will merge that into hipo-dev. She's working also on BLAS detection.

[galabovaa]

I added the julia build and workflows to hipo-tt and the tests are passing now. @odow, can you please have a look and double check that things there work as expected? I looked up how blas trampoline would work and found that a suggested way was to use BLA_VENDOR so I added this to our CMake for the BLAS, is that OK?

[odow]

See the builds https://github.com/ERGO-Code/HiGHS/actions/runs/18570757342/job/52943747516

Mac and linux are finding the host BLAS:

We need them to find the BLAS in $prefix, like Windows does:

We cannot assume that the machines we deploy the binaries on have a BLAS in the same location.

[odow]

I don't really see the need for

HiGHS/cmake/FindHipoDeps.cmake

Lines 5 to 109 in fb5e7cf

	set(BLA_VENDOR "" CACHE STRING "For blas trampoline")
	# Optionally set the vendor:
	# set(BLA_VENDOR libblastrampoline)
	if (BLA_VENDOR STREQUAL "")
	if (WIN32)
	if (NOT (BLAS_ROOT STREQUAL ""))
	message(STATUS "Looking for blas in " ${BLAS_ROOT})
	set(OpenBLAS_ROOT ${BLAS_ROOT})
	message(STATUS "OpenBLAS_ROOT is ${OpenBLAS_ROOT} ")
	find_package(OpenBLAS CONFIG NO_DEFAULT_PATH)
	if(OpenBLAS_FOUND)
	message(STATUS "OpenBLAS CMake config path: ${OpenBLAS_DIR}")
	else()
	message(STATUS "OpenBLAS not found in ${BLAS_ROOT}")
	endif()
	endif()
	if ((BLAS_ROOT STREQUAL "") OR (NOT OpenBLAS_FOUND))
	# (NOT OpenBLAS_FOUND AND NOT BLAS_FOUND))
	message(STATUS "Looking for blas")
	find_package(OpenBLAS REQUIRED)
	if(OpenBLAS_FOUND)
	if(TARGET OpenBLAS::OpenBLAS)
	message(STATUS "OpenBLAS CMake config path: ${OpenBLAS_DIR}")
	elseif(OPENBLAS_LIB)
	message(STATUS "Linking against OpenBLAS via raw library: ${OPENBLAS_LIB}")
	else()
	# try blas
	# find_package(BLAS)
	# if (BLAS_FOUND)
	# message(STATUS "Using BLAS library: ${BLAS_LIBRARIES}")
	# message(STATUS "BLAS include dirs: ${BLAS_INCLUDE_DIRS}")
	# else()
	# message(STATUS "OpenBLAS found but no target?")
	# endif()
	endif()
	else()
	message(FATAL_ERROR "No BLAS library found")
	endif()
	endif()
	elseif(NOT APPLE)
	# LINUX
	# If a BLAS install was specified try to use it first.
	if (NOT (BLAS_ROOT STREQUAL ""))
	message(STATUS "Looking for blas in " ${BLAS_ROOT})
	find_library(OPENBLAS_LIB
	NAMES openblas
	HINTS "${BLAS_ROOT}/lib"
	NO_DEFAULT_PATH)
	if(OPENBLAS_LIB)
	message("Found OpenBLAS library at ${OPENBLAS_LIB}")
	else()
	find_library(BLAS_LIB
	NAMES blas
	HINTS "${BLAS_ROOT}/lib"
	NO_DEFAULT_PATH)
	if(BLAS_LIB)
	message("Found BLAS library at ${BLAS_LIB}")
	else()
	message("Did not find blas library at ${BLAS_ROOT}")
	message("Attempting default locations search")
	endif()
	endif()
	endif()
	if ((BLAS_ROOT STREQUAL "") OR (NOT OPENBLAS_LIB AND NOT BLAS_LIB))
	find_library(OPENBLAS_LIB
	NAMES openblas
	HINTS "${BLAS_ROOT}/lib")
	if(OPENBLAS_LIB)
	message("Found OpenBLAS library at ${OPENBLAS_LIB}")
	else()
	find_library(BLAS_LIB
	NAMES blas
	HINTS "${BLAS_ROOT}/lib")
	if(BLAS_LIB)
	message("Found BLAS library at ${BLAS_LIB}")
	else()
	message(FATAL_ERROR "No BLAS library found")
	endif()
	endif()
	endif()
	endif()
	else()
	# if (NOT BLA_VENDOR STREQUALS "")
	find_package(BLAS REQUIRED)
	if (BLAS_FOUND)
	message(STATUS "Using BLAS library: ${BLAS_LIBRARIES}")
	message(STATUS "BLAS include dirs: ${BLAS_INCLUDE_DIRS}")
	else()
	message(FATAL_ERROR "No BLAS library found!")
	endif()
	endif()

What's the problem if we just do

https://github.com/ERGO-Code/HiGHS/blob/fb5e7cf040b2a9e10d25fa1febcc6ff9cbe54e53/cmake/FindHipoDeps.cmake#L102C5-L108

and then

HiGHS/highs/CMakeLists.txt

Lines 219 to 227 in fb5e7cf

	# BLA_VENDOR Specified
	target_link_libraries(highs PRIVATE BLAS::BLAS)
	string(TOLOWER "${BLAS_LIBRARIES}" blas_lower)
	if(blas_lower MATCHES "openblas")
	target_compile_definitions(highs PRIVATE HIPO_USES_OPENBLAS)
	elseif(blas_lower MATCHES "accelerate")
	target_compile_definitions(highs PRIVATE HIPO_USES_APPLE_BLAS)
	endif()

Is there some issue if a downstream project uses HiGHS in its CMake?

[odow]

@galabovaa in the Julia build I just forced the BLAS I wanted

[odow]

Stepping back, what are the use cases we're trying to support?

It seems you want the defaults to work if:

1. Someone on linux who has done sudo apt install libopenblas-dev
2. Someone is on macOS and has accelerate
3. Someone on Windows if they have vcpkg install openblas and provided the vcpkg CMake thing

We also want to support:

4. cross-compiling from linux to any platform (e.g., Julia)
5. someone on Mac and using a BLAS other than accelerate
6. someone on linux and using MKL?

The last three can be supported by someone explicitly setting the BLAS library, like I'm doing for Julia in this PR?

And 1. and 2. and 3. are supported by find_package(BLAS, required)?

[galabovaa]

Sure, find_package(BLAS) is probably enough. Will leave the other code as optional, for now.

It would be great if you could pull the updates from hipo-dev and double check that the julia side is OK

[odow]

CI runs look okay to me