conda-forge / libosqp-feedstock

A conda-smithy repository for libosqp.
BSD 3-Clause "New" or "Revised" License
0 stars 5 forks source link

Build against libqdldl #8

Closed h-vetinari closed 1 year ago

conda-forge-linter commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

h-vetinari commented 1 year ago

@traversaro Another old project here - but I'm quickly reaching points where it gets painful. The osqp/qdldl stack is an unsexy onion of many different nested layers, but not always the same.

For example, there are vendored (but modified!) sources from https://github.com/DrTimothyAldenDavis/SuiteSparse/tree/master/AMD both in

I think it might have been a mistake to install them in the environment-wide $PREFIX/lib in https://github.com/conda-forge/qdldl-python-feedstock/pull/14, but at least the artefact is named differently here.

The next problem is that osqp tries to only build OBJECT libraries (and obscure third type that's neither shared nor static), presumably so that they can then be placed into the final artefact without the cost of indirecting through the DLL-boundary(?).

Not sure if/how we want to untangle this. I have some more WIP patches on the osqp sources, if you're interested, but wanted to discuss a bit first before sinking more time into this...

traversaro commented 1 year ago

For example, there are vendored (but modified!) sources from https://github.com/DrTimothyAldenDavis/SuiteSparse/tree/master/AMD both in

* https://github.com/osqp/qdldl-python/tree/master/c/amd

* https://github.com/osqp/osqp/tree/master/lin_sys/direct/qdldl/amd

I think it might have been a mistake to install them in the environment-wide $PREFIX/lib in conda-forge/qdldl-python-feedstock#14, but at least the artefact is named differently here.

Ah great, I saw the amd vendoring back in time (https://github.com/conda-forge/libosqp-feedstock/issues/3#issue-781125353) but I had missed the "modified" part. Yes, ideally they should have symbols with a different name.

The next problem is that osqp tries to only build OBJECT libraries (and obscure third type that's neither shared nor static), presumably so that they can then be placed into the final artefact without the cost of indirecting through the DLL-boundary(?).

Yes, I think OBJECT is used here for two reason:

However, I do not think this makes the de-vendoring more complex (I hope at least). It should be sufficient to just remove the mention of qdldl in:

and then add a normal link to the qdldl shared library in:

traversaro commented 1 year ago

I forgot another important information: upstream osqp changed a lot its CMake build system (in branch https://github.com/osqp/osqp/blob/develop-1.0), and that version will be eventually released as osqp 1.0 . I guess any de-vendoring done now may need to be updated substantially for that version.

h-vetinari commented 1 year ago

Sorry for the delay in getting back to this. I just got back to the initial problem I was trying to solve that had prompted me to look at the stack again (in the context of the various migrations...): https://github.com/conda-forge/osqp-feedstock/pull/48

h-vetinari commented 1 year ago

@traversaro This is finally getting somewhere!

I've left a longer comment in the commit I came up with, but in short: rather than mucking about with the OBJECT files or a separate library, I'm just pulling the glue-code directly into the osqp-sources (where they arguably belong anyway).

What I've left by the wayside here is the mkl pardiso integration, but this was broken already (as we don't have a host dep, and the libname that's hardcoded upstream doesn't match ours).

Seeing that osqp 1.0 will refactor this quite significantly, I think it's OK to rectify this for 1.0 (then we should also have different outputs for the python bindings, which the most recent version does already).

I feel pretty good about this PR now, but would very much appreciate your review! :)

traversaro commented 1 year ago

Thanks a lot for your work @h-vetinari !