scipopt / russcip

Rust interface for SCIP
https://crates.io/crates/russcip
Apache License 2.0
30 stars 10 forks source link

Automatically fetch and compile SCIP #125

Closed jacobsvante closed 3 months ago

jacobsvante commented 5 months ago

Now that SCIP is using the Apache license, it would be fitting to automatically fetch and compile the SCIP library when you cargo install this crate. This should make it way more accessible to people.

What do you think? Is this feasible?

mmghannam commented 5 months ago

Hi @jacobsvante! Yes, that is of course a valid request but I opted not to do that for two reasons:

  1. SCIP is a complex piece of software that has a lot of dependencies and can be a pain to get to work, let alone do it in an automated way.
  2. and, more importantly, it would make the compilation time so much more (minutes for the first time) than it is right now.

That being said, I think the goal of making russcip (and SCIP) more accessible is to do a similar approach to python packages; i.e. fetch a prebuilt binary of SCIP (and its dependencies) on the fly. As of right now, prebuilt binaries exist for the 3 OSs but only on intel x86 architecture. @Opt-Mucca is currently working on extending that to ARM. Once that is done, I plan to add it as a fallback option in case neither SCIPOPTDIR is provided or conda environment is active (with SCIP installed).

jacobsvante commented 5 months ago

Hi @mmghannam,

Thanks for the response.

I don't think I've heard of including pre-built binaries in a crate before. Do you know of any crate that does this?

Where can I follow @Opt-Mucca's work? Arm-support would be a very good addition!

Opt-Mucca commented 5 months ago

Hey @jacobsvante

Currently I do most of the work offline. For Windows and MacOS w/ x86 I build using GitHub Actions on Mo's fork of SCIP: https://github.com/mmghannam/scip/tree/mt/build_from_source For linux I use docker containers provided by manylinux2014

The main reason I'm doing this is so SCIP can be automatically packaged when using pip for Python (which now is done for the most part).

Right now I'm having trouble with MacOS on the new ARM processors, although I think that's simply because I haven't sat down with one for a few hours. As for ARM on linux, I wasn't planning on doing anything until I saw that there was a demand.

jacobsvante commented 5 months ago

Sounds like you're making good strides, keep it up @Opt-Mucca! 😃

mmghannam commented 4 months ago

Thanks to @Opt-Mucca's efforts, precompiled binaries are released for SCIP 9.0.0 for all OS's on x86 and arm too for MacOS. I released today a new version of russcip that has a bundled feature that automatically downloads the precompiled binaries and links against them. @jacobsvante What do you think? I'm still more on the side of not adding the complexity of compiling SCIP to the scip-sys repo, and I think this will make SCIP/russcip accessible to most people, and even more OS-architecture combinations could be added in the future. I'd be curious to know if it works for your use case.

jacobsvante commented 3 months ago

Sweet! Good job!

I tried it out just now but the scip.zib.de domain is not working for me at the moment.

Keeping it in the russcip library works for me, I'm using it through good-lp.

I'll get back once I can confirm if it's working for me. (I'm on a M3 Apple silicon MacBook Pro).

mmghannam commented 3 months ago

Thanks for the feedback, maybe we can move the files to Github to avoid these issues.

jacobsvante commented 3 months ago

Sounds like a good idea. Or you could use that as a mirror.

mmghannam commented 3 months ago

I updated scip-sys with the github links and made a new release of russcip. I'd like to close this issue for now as I think this alternative solution addresses the accessibility issue for most people. @jacobsvante please feel free to reopen if you still think there's a need.

jacobsvante commented 3 months ago

Nice @mmghannam!

One issue though.

I copy-pasted the russcip example to a new project and added russcip 0.3.2 (cargo add russcip -F bundled). Running with cargo run works great. But when trying to run the generated binary directly I get the following error:

❯ ./target/debug/russcip-example
dyld[43720]: Library not loaded: libscip.9.0.dylib
  Referenced from: <9DE8235D-4131-3A28-B724-37A3D12D8480> /Users/jacob/apps/jacobs-sandbox/russcip-example/target/debug/russcip-example
  Reason: tried: 'libscip.9.0.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OSlibscip.9.0.dylib' (no such file), 'libscip.9.0.dylib' (no such file), '/Users/jacob/apps/jacobs-sandbox/russcip-example/libscip.9.0.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/jacob/apps/jacobs-sandbox/russcip-example/libscip.9.0.dylib' (no such file), '/Users/jacob/apps/jacobs-sandbox/russcip-example/libscip.9.0.dylib' (no such file)
[1]    43720 abort      ./target/debug/russcip-example
mmghannam commented 3 months ago

Oh I haven't thought about this use case. I think this is because I'm linking to libscip dynamically. But that should be fixable if statically built binaries are used. I added this issue in scip-sys https://github.com/scipopt/scip-sys/issues/10.

jacobsvante commented 3 months ago

Thanks @mmghannam

mwien commented 3 months ago

Oh I haven't thought about this use case. I think this is because I'm linking to libscip dynamically. But that should be fixable if statically built binaries are used. I added this issue in scip-sys scipopt/scip-sys#10.

Not very knowledgeable, but maybe another way would be to set the rpath of the executable similar to here (because the library files are somewhere in the build folder, the executable just does not find them) 🤔 https://github.com/Rust-SDL2/rust-sdl2#bundled-feature

On another note: I saw the PR regarding static linking, I would be interested in that for independent reasons 😊 it seems that the pre-built binaries aren't compiled with the correct flags to make this possible? So only option would be to have static linking with a "self-built" SCIP installation somewhere on the system (in SCIPOPTDIR).

Anyway, thanks for your effort in making SCIP accessible from Rust :)

mmghannam commented 3 months ago

Oh I haven't thought about this use case. I think this is because I'm linking to libscip dynamically. But that should be fixable if statically built binaries are used. I added this issue in scip-sys scipopt/scip-sys#10.

Not very knowledgeable, but maybe another way would be to set the rpath of the executable similar to here (because the library files are somewhere in the build folder, the executable just does not find them) 🤔 https://github.com/Rust-SDL2/rust-sdl2#bundled-feature

I'm doing that in scip-sys in this line, but I'm definitely missing something there. From what I understand, the link you shared compiles and distributes the sdl2 library files, which is not really the use case for russcip. We just want the executable to be able to find the library files at runtime.

On another note: I saw the PR regarding static linking, I would be interested in that for independent reasons 😊 it seems that the pre-built binaries aren't compiled with the correct flags to make this possible? So only option would be to have static linking with a "self-built" SCIP installation somewhere on the system (in SCIPOPTDIR).

The build scripts are in this repo in this folder. I don't have a lot of experience with this, I compiled SCIP with -DSHARED=false but that doesn't seem to be enough. Maybe you could take a look there and add a PR if you have the time? and I'll happily change the bundled files and update scip-sys.

Anyway, thanks for your effort in making SCIP accessible from Rust :)

Thanks, I appreciate it :)

mwien commented 3 months ago

I'm doing that in scip-sys in this line, but I'm definitely missing something there. From what I understand, the link you shared compiles and distributes the sdl2 library files, which is not really the use case for russcip. We just want the executable to be able to find the library files at runtime.

Yeah, true. Then, from what I gather using rpath is not really feasible. Apparently there was a period where people exploited that rustc-link-arg propagated transitively up to the final binary, but that was changed... however, could also be that I misunderstand this...

The build scripts are in this repo in this folder. I don't have a lot of experience with this, I compiled SCIP with -DSHARED=false but that doesn't seem to be enough. Maybe you could take a look there and add a PR if you have the time? and I'll happily change the bundled files and update scip-sys.

Yes, will very likely try to get this to work, can't promise on when though (or on whether I will succeed).

mwien commented 3 months ago

For static linking, maybe something like this can help if we want to avoid recompilation: https://github.com/rust-lang/rust/issues/35061.

At least having flag "-C relocation-model=dynamic-no-pic" gives me different errors (missing libraries)...

mmghannam commented 3 months ago

@mwien thanks for your help! I always get frustrated quickly when working on the build system, but unfortunately it's necessary. I tried to avoid it till now but it seems that compiling SCIP from source is probably the best option to fit most cases. I should've just followed @jacobsvante advice from the start 😅 but at least now we have this bundled feature. I'm now working on a feature in scip-sys in this PR https://github.com/scipopt/scip-sys/pull/14 to do just that. Turns out the compile times are not as bad as I thought and the cmake crate is easy to use. I managed to get it working locally (on macos-arm) and once it's working well in CI, I will publish it and add corresponding features to russcip.

jacobsvante commented 3 months ago

Tjihoo! Nice @mmghannam!

mmghannam commented 3 months ago

Ok I managed to get CI to work for macos and ubuntu, I suspect windows works too but just tbb is needed on the system. I just published a new version of russcip that has the new from-source feature. @jacobsvante @mwien It would be great if you could try it and let me know if it works for you. This is still a first version that probably wouldn't work for everyone out of the box but I'll close the issue for now and if there are problems I suggest we solve them in following issues.