Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use precompiled JLL #7

Merged
merged 1 commit into from
Jul 6, 2021
Merged

use precompiled JLL #7

merged 1 commit into from
Jul 6, 2021

Conversation

dpo
Copy link
Contributor

@dpo dpo commented Jun 21, 2021

I prepared precompiled binaries for QPALM_vLADEL for a variety of flavors of linux, macOS, FreeBSD and Windows in the form of a JLL package: https://github.com/JuliaBinaryWrappers/QPALM_jll.j (you can see the available platforms here: https://github.com/JuliaBinaryWrappers/QPALM_jll.jl/releases/tag/QPALM-v0.1.1+0).

This PR makes it seamless for users to install QPALM.jl and have the appropriate binaries installed automatically as Julia artifacts.

On macOS, I observe two test failures:

Inequalities (4 active): Test Failed at /Users/dpo/dev/julia/QPALM.jl/test/feasible.jl:38
  Expression: maximum(max.(A * results.x - b, 0.0)) <= 0.0001
   Evaluated: 0.00040231975711657597 <= 0.0001
Stacktrace:
 [1] macro expansion
   @ ~/dev/julia/QPALM.jl/test/feasible.jl:38 [inlined]
 [2] macro expansion
   @ /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1226 [inlined]
 [3] macro expansion
   @ ~/dev/julia/QPALM.jl/test/feasible.jl:14 [inlined]
 [4] macro expansion
   @ /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1151 [inlined]
 [5] top-level scope
   @ ~/dev/julia/QPALM.jl/test/feasible.jl:5
Inequalities (4 active): Test Failed at /Users/dpo/dev/julia/QPALM.jl/test/feasible.jl:39
  Expression: abs(dot(results.y, A * results.x - b)) <= 0.0001
   Evaluated: 0.00021791420910928668 <= 0.0001
Stacktrace:
 [1] macro expansion
   @ ~/dev/julia/QPALM.jl/test/feasible.jl:39 [inlined]
 [2] macro expansion
   @ /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1226 [inlined]
 [3] macro expansion
   @ ~/dev/julia/QPALM.jl/test/feasible.jl:14 [inlined]
 [4] macro expansion
   @ /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1151 [inlined]
 [5] top-level scope
   @ ~/dev/julia/QPALM.jl/test/feasible.jl:5

The tolerances are nearly attained. I built QPALM_vLADEL against OpenBLAS. Perhaps that matters?!

@Benny44
Copy link
Collaborator

Benny44 commented Jun 21, 2021

Hi Dominique, thank you very much for this contribution. The failures on MacOS are likely because QPALM uses a relative and absolute tolerance (both default to 1e-4). I set the relative to 0 now in the tests. Could you please run them again and confirm whether it works, then I will integrate your pull request. (Sorry, I don't have a Mac, and travis made their service paid so I can't test it.)

@dpo
Copy link
Contributor Author

dpo commented Jun 21, 2021

Yes that seems to fix it. Now I'm seeing output of the form

Iter |   P. res   |   D. res   |  Stepsize  |  Objective
==========================================================
   0 | ---------------------------------------------------
   1 | 0.0000e+00 | 1.0000e+00 | 1.0000e+00 | -1.0000e+08
   2 | 7.7486e-297 | 1.0000e+00 | 1.0000e+00 | -1.0000e+08


=============================================================
| QPALM detected a dual infeasible problem. You can check   |
| the certificate of this infeasiblity. If you think the    |
| problem might not be dual infeasible, try lowering the    |
| infeasiblity tolerance eps_dual_inf.                      |
| runtime:         0.04 milliseconds                        |
=============================================================

with a suspicious final residual, and I get a segmentation fault in the warm start tests:

signal (11): Segmentation fault: 11
in expression starting at /Users/dpo/dev/julia/QPALM.jl/test/warm_start.jl:3
prea_vec_copy at /Users/dpo/.julia/artifacts/3125b3dbabd8068f23ebb27beec238e6f375f368/lib/libqpalm.dylib (unknown line)
qpalm_warm_start at /Users/dpo/.julia/artifacts/3125b3dbabd8068f23ebb27beec238e6f375f368/lib/libqpalm.dylib (unknown line)
#warm_start!#6 at /Users/dpo/dev/julia/QPALM.jl/src/wrappers.jl:295 [inlined]
warm_start!##kw at /Users/dpo/dev/julia/QPALM.jl/src/wrappers.jl:291
unknown function (ip: 0x1115835b8)

that I can reproduce on linux.

The link to the Bintray binaries that is in the documentation gives me a 404, which was the original motivation for building the JLL. Do the warm start tests pass for you?

Here are the commands I used to compile QPALM: https://github.com/JuliaPackaging/Yggdrasil/blob/master/Q/QPALM/build_tarballs.jl#L31. Does anything seem off to you?

@dpo dpo mentioned this pull request Jun 21, 2021
@Benny44
Copy link
Collaborator

Benny44 commented Jun 21, 2021

The warm start function has been updated and needs to be one call now (since I set work->x/y to NULL in C and check for that later if the corresponding x/y_warm_start is NULL). Actually, that would have never worked, since with sequential warm start calls, the first variable to be set would have been overwritten by 0 in the second call. Anyways, I fixed the test, and this should run now. Can you confirm?

@dpo
Copy link
Contributor Author

dpo commented Jun 21, 2021

All green lights on macOS and Linux!

There's a segfault on Windows, but I don't know if anyone's tested the code there.

@dpo
Copy link
Contributor Author

dpo commented Jun 21, 2021

Also it might make sense to move qpalm_julia.c to QPALM_vLADEL/interfaces/QPALM.jl/. That's what I did to compile the JLL. It's more useful there than here at this point.

@Benny44
Copy link
Collaborator

Benny44 commented Jun 22, 2021

I don't understand your last comment. QPALM.jl is a submodule in QPALM_vLADEL, so qpalm_julia.c is already located in this folder, no?

@dpo
Copy link
Contributor Author

dpo commented Jun 22, 2021

QPALM.jl is a submodule in QPALM_vLADEL

I'm saying that that's no longer necessary. With this PR, the C library and the Julia module can be separated since users no longer need to clone either repository to use the Julia interface.

@lostella
Copy link
Member

lostella commented Jul 6, 2021

Would it make sense to merge this and sort out the issues on Windows separately?

@Benny44
Copy link
Collaborator

Benny44 commented Jul 6, 2021

Sorry guys, I wanted to have a look at the CI since I have to fix it anyways, but got distracted since it was my public defense today. On the bright side, the issues on Windows should have been fixed. Turned out to be a different definition of the type long there.

@Benny44 Benny44 merged commit 00f9646 into kul-optec:master Jul 6, 2021
@dpo
Copy link
Contributor Author

dpo commented Jul 6, 2021

got distracted since it was my public defense today.

congratulations on your defense!

the issues on Windows should have been fixed

where was that fixed?

@dpo dpo deleted the jll branch July 6, 2021 17:59
@Benny44
Copy link
Collaborator

Benny44 commented Jul 6, 2021

Thanks.

In the latest couple of commits of https://github.com/Benny44/LADEL and https://github.com/Benny44/QPALM_vLADEL/

@lostella
Copy link
Member

lostella commented Jul 6, 2021

but got distracted since it was my public defense today

woah, congrats!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants