Chuffed: initial commit #6

Kieranoski702 · 2023-10-02T15:38:57Z

This PR copies previous work on these bindings (in a private repository) into the repo. This is a proof of concept currently, and has no functionality apart from calling chuffed's build system, linking with the generated static library (libchuffed.a), and generating the automatic bindings using bindgen.

The next steps for this are:

Document chuffed's existing API
To setup some basic CI
Allow the functions, constants, etc. required to create a real example such as solving a sudoku puzzle

yigityazicilar · 2023-10-02T16:36:56Z

This is more a general thing across solvers but I will also list it here so that it can be applied in the future. It might be good to include

#![allow(non_upper_case_globals)]
#![allow(non_camel_case_types)]
#![allow(non_snake_case)]

at the top of lib.rs files so that Rust does not complain about the C/C++ code not fitting Rust conventions.

niklasdewally · 2023-10-02T17:05:02Z

@OcEaNvS is there a way to do this by module, or is it by function or by crate only?

The C++ bindings (in my case at least) are their own module, so if it is possible it would be nice to make it module only :).

yigityazicilar · 2023-10-02T18:39:04Z

@niklasdewally When you add those to the lib.rs of your module it stops Rust complaining about the generated bindings when they are included. This effect happens module wide. https://rust-lang.github.io/rust-bindgen/tutorial-4.html

yigityazicilar · 2023-10-02T18:43:14Z

@Kieranoski702 I am getting a linking error while using the createVar function. Before trying to use any of the C++ functions this library compiled without any problems. I am not sure if this is just an error on my side but I am including the error message in case it is not.

  = note: /usr/bin/ld: ./solvers/chuffed/vendor/build//libchuffed.a(engine.cpp.o): in function `Engine::search(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)':
          engine.cpp:(.text+0x3312): undefined reference to `FlatZinc::FlatZincSpace::onRestart(Engine*)'
          /usr/bin/ld: engine.cpp:(.text+0x3d2f): undefined reference to `FlatZinc::FlatZincSpace::onRestart(Engine*)'
          /usr/bin/ld: engine.cpp:(.text+0x4ee8): undefined reference to `FlatZinc::FlatZincSpace::onRestart(Engine*)'
          /usr/bin/ld: ./solvers/chuffed/vendor/build//libchuffed.a(engine.cpp.o): in function `Engine::constrain()':
          engine.cpp:(.text._ZN6Engine9constrainEv[_ZN6Engine9constrainEv]+0x88): undefined reference to `FlatZinc::FlatZincSpace::storeSolution()'
          /usr/bin/ld: engine.cpp:(.text._ZN6Engine9constrainEv[_ZN6Engine9constrainEv]+0x157): undefined reference to `FlatZinc::FlatZincSpace::onRestart(Engine*)'
          collect2: error: ld returned 1 exit status

To check if this is just on my side you can add .allowlist_function(createVar) to ./solvers/chuffed/build.rs and change ./solvers/chuffed/src/main.rs to the following:

use chuffed_rs::bindings::createVar;
use chuffed_rs::bindings::IntVar;

pub fn main() {
    let mut var = std::mem::MaybeUninit::<IntVar>::uninit();
    unsafe {
        createVar(&mut var.as_mut_ptr(), 0, 5, false);
    }
}

yigityazicilar · 2023-10-02T16:42:06Z

solvers/chuffed/src/lib.rs

@@ -0,0 +1,5 @@
+pub mod bindings {


Suggested change

pub mod bindings {

#![allow(non_upper_case_globals)]

#![allow(non_camel_case_types)]

#![allow(non_snake_case)]

pub mod bindings {

yigityazicilar · 2023-10-03T10:27:44Z

solvers/chuffed/build.rs

+    // from project root
+    println!("cargo:rustc-link-search=all=./solvers/chuffed/vendor/build/");
+    println!("cargo:rustc-link-lib=static=chuffed");
+


The linking issue can be fixed by adding the following code.

Suggested change

println!("cargo:rustc-link-lib=static=chuffed_fzn");

The chuffed solver is tightly integrated with the flatzinc modelling language. The flatzinc files are not included in the libchuffed.a library. These files get compiled to a separate library called libchuffed_fzn.a.

I've added this along with your suggested changes to lib.rs. I've also tried to run your example main function after putting createVar into the allow list in build.rs. I get a different error from you when trying to compile now even after adding the link to libchuffed_fzn.a. Were you able to get your example main running after you made those changes?

Could you send the error that you are getting so that I can look into it? I was able to get it to run on my end.

Could you send the error that you are getting so that I can look into it? I was able to get it to run on my end.

Yeah sure thing, it's quite a lot so just gonna show a snippet

error: linking with `cc` failed: exit status: 1 | = note: LC_ALL="C" PATH="/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin:/home/kf77/.opam/default/bin:/home/kf77/local/bin:/home/kf77/local/bin:/home/kf77/local/bin:/home/kf77/local/bin:/home/kf77/.cargo/bin:/home/kf77/.cabal/bin:/home/kf77/.ghcup/bin:/home/kf77/.local/bin:/home/kf77/bin:/usr/share/Modules/bin:/usr/local/julia/bin:/home/kf77/usr/bin:/cs/home/kf77/usr/bin:/usr/local/maven/bin:/usr/local/amazon-corretto-17/bin:/usr/lib64/ccache:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/home/kf77/.dotnet/tools" VSLANG="1033" "cc" "-m64" "/tmp/rustcyP3VVJ/symbols.o" "/home/kf77/conjure-oxide/target/debug/deps/chuffed_rs-3916e90f286e3739.38o4ig1bt7kkyjdv.rcgu.o" "/home/kf77/conjure-oxide/target/debug/deps/chuffed_rs-3916e90f286e3739.3po8wt4hdebzumrt.rcgu.o" "/home/kf77/conjure-oxide/target/debug/deps/chuffed_rs-3916e90f286e3739.4jresi8mttp46v6o.rcgu.o" "/home/kf77/conjure-oxide/target/debug/deps/chuffed_rs-3916e90f286e3739.4jyq2iwe1v1mymye.rcgu.o" "/home/kf77/conjure-oxide/target/debug/deps/chuffed_rs-3916e90f286e3739.520za8226h1pu3p6.rcgu.o" "/home/kf77/conjure-oxide/target/debug/deps/chuffed_rs-3916e90f286e3739.5brh85kw9qop90tt.rcgu.o" "-Wl,--as-needed" "-L" "/home/kf77/conjure-oxide/target/debug/deps" "-L" "./solvers/chuffed/vendor/build/" "-L" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bstatic" "/home/kf77/conjure-oxide/target/debug/deps/libchuffed_rs-d37182f69c381980.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-b850d2b001350814.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libpanic_unwind-a3d926e3ce38f6cc.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libobject-0a660adf51e0d01c.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libmemchr-49252de8bfa771e2.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libaddr2line-6f52a21444f37c70.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libgimli-57e8575f0ca731a2.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_demangle-da4065b9a8ae7be4.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd_detect-1ad47776352198ff.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libhashbrown-7075f8a8dc932a7c.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_alloc-a494eb4ddbb000f0.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libminiz_oxide-672778ed8628861b.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libadler-9bba1c59dcc21440.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libunwind-b9849fb628ce12eb.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcfg_if-0ea4fcc08eb96f77.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liblibc-a8ba743c059cc198.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liballoc-e4195ee2443e1656.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_core-04991604e2730fd6.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-f02db372677d4667.rlib" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-448eba810c83029d.rlib" "-Wl,-Bdynamic" "-lstdc++" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/home/kf77/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "/home/kf77/conjure-oxide/target/debug/deps/chuffed_rs-3916e90f286e3739" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-nodefaultlibs" = note: /usr/bin/ld: /home/kf77/conjure-oxide/target/debug/deps/libchuffed_rs-d37182f69c381980.rlib(modelling.cpp.o): relocation R_X86_64_32 against symbol `sat' can not be used when making a PIE object; recompile with -fPIE /usr/bin/ld: /home/kf77/conjure-oxide/target/debug/deps/libchuffed_rs-d37182f69c381980.rlib(bool-view.cpp.o): relocation R_X86_64_32 against `.LC4' can not be used when making a PIE object; recompile with -fPIE /usr/bin/ld: /home/kf77/conjure-oxide/target/debug/deps/libchuffed_rs-d37182f69c381980.rlib(sat.cpp.o): relocation R_X86_64_32 against `.LC3' can not be used when making a PIE object; recompile with -fPIE /usr/bin/ld: /home/kf77/conjure-oxide/target/debug/deps/libchuffed_rs-d37182f69c381980.rlib(conflict.cpp.o): relocation R_X86_64_32 against `.LC1' can not be used when making a PIE object; recompile with -fPIE /usr/bin/ld: /home/kf77/conjure-oxide/target/debug/deps/libchuffed_rs-d37182f69c381980.rlib(int-var.cpp.o): relocation R_X86_64_32S against `.rodata' can not be used when making a PIE object; recompile with -fPIE /usr/bin/ld: /home/kf77/conjure-oxide/target/debug/deps/libchuffed_rs-d37182f69c381980.rlib(int-var-el.cpp.o): relocation R_X86_64_32 against `.LC3' can not be used when making a PIE object; recompile with -fPIE /usr/bin/ld: /home/kf77/conjure-oxide/target/debug/deps/libchuffed_rs-d37182f69c381980.rlib(int-var-sl.cpp.o): relocation R_X86_64_32S against symbol `_ZN10Propagator6wakeupEii' can not be used when making a PIE object; recompile with -fPIE /usr/bin/ld: /home/kf77/conjure-oxide/target/debug/deps/libchuffed_rs-d37182f69c381980.rlib(int-var-ll.cpp.o): relocation R_X86_64_32 against `.LC0' can not be used when making a PIE object; recompile with -fPIE /usr/bin/ld: /home/kf77/conjure-oxide/target/debug/deps/libchuffed_rs-d37182f69c381980.rlib(ldsb.cpp.o): relocation R_X86_64_32 against symbol `sat' can not be used when making a PIE object; recompile with -fPIE

Hmmmm, that seems weird. I am not having any issues on my side. Let me clone your branch and try again.

For history's sake, this is due to the issues with AlmaLinux discussed here: #6 (comment)

ozgurakgun · 2023-10-03T12:09:44Z

Are we able to add a simple CI test for this?

Kieranoski702 · 2023-10-03T20:28:02Z

Are we able to add a simple CI test for this?

I'll add some CI tests ASAP but me, Nik and Yigit discovered some issues when building on the lab machines (AlmaLinux) that might have implications for CI tests.

This fork builds on Ubuntu, Arch and Mac but the lab machines seem to have issues linking libchuffed.a/libchuffed_fzn.a. Nik spun up a VM of AlmaLinux, installed cmake, build essentials, clang, rust, etc and encountered the same problem so it seems OS specific not specific to the lab machines specifically.

I was thinking of adding some CI build tests for Ubuntu, Arch, Mac as well as some others but wondered if you think it's a good idea to put effort into fixing the build issues on AlmaLinux or just accept that it builds on most platforms for now?

niklasdewally · 2023-10-03T20:57:16Z

For clarification, minion and other things build fine on alma, it is specifically chuffed.

yigityazicilar · 2023-10-03T22:26:04Z

After more research the Null Pointer way is how I believe that functions that take double pointers should be wrapped. Previously, I looked into MaybeUninit but that does not get used for its intended purpose after returning the mutable pointer using the .as_mut_ptr() function. When the pointer that is returned is initialized using the C++ function the .assume_init() function inside MaybeUninit returns random memory. I am still including the MaybeUninit version that does not use .assume_init() as a reference. Please let me know if you have any other ideas on how to correctly do this. @ChrisJefferson @ozgurakgun

Null Pointer way

// The signature of createVar is below for reference.
// createVar(x: *mut *mut IntVar, min: ::std::os::raw::c_int, max: ::std::os::raw::c_int, el: bool)
pub fn create_var(min: i32, max: i32, el: bool) -> Box<IntVar> {
    let mut ptr: *mut IntVar = ptr::null_mut();

    unsafe {
        createVar(&mut ptr, min, max, el);
        return Box::from_raw(ptr);
    }
}

MaybeUninit way

// The signature of createVar is below for reference.
// createVar(x: *mut *mut IntVar, min: ::std::os::raw::c_int, max: ::std::os::raw::c_int, el: bool)
pub fn create_var(min: i32, max: i32, el: bool) -> Box<IntVar> {
    let mut uninit: MaybeUninit<IntVar> = MaybeUninit::uninit();
    let mut ptr = uninit.as_mut_ptr();

    unsafe {
        createVar(&mut ptr, min, max, el);
        return Box::from_raw(ptr);
    }
}

ChrisJefferson · 2023-10-04T01:23:40Z

Hi, replying on my phone as away from computer until Monday! Am happy to look more when I am back.

The null pointer feels more right in this case, as the C++ code is using the double pointer because what it actually wants is to give you its own pointer -- so it needs to take a pointer to pointer so it can change the inner pointer.

I'm not actually sure it is safe to the put that pointer in a Box, as that will, I believe, free the memory pointed to when the Box goes out of scope, but freeing memory allocated when creating C++ classes via a rust box might not work. It will certainly break in some situations, not sure about here. I would make a custom Var type and shove the pointer in there, just leak it for now (does chuffed even support freeing variables? Not sure)

niklasdewally · 2023-10-04T13:03:57Z

@Kieranoski702 @yigityazicilar I have moved the Alma issues into #9.

Also breaks on fedora, but with a different message?

niklasdewally

What about Cargo Test on CI?

niklasdewally

LGTM 🚀

Chuffed: initial commit

901c79d

yigityazicilar suggested changes Oct 3, 2023

View reviewed changes

Kieranoski702 and others added 2 commits October 3, 2023 16:08

Add allows for non standard function naming, add example main

382ee14

Add git submodule to build.sh and move allows in lib.rs

f4b009b

Kieranoski702 mentioned this pull request Oct 3, 2023

Chuffed Bindings #8

Closed

4 tasks

niklasdewally mentioned this pull request Oct 4, 2023

Chuffed: linking errors on Alma Linux and Fedora #9

Closed

Add basic CI tests based on minion-tests.yml

b110426

This comment was marked as duplicate.

Sign in to view

niklasdewally approved these changes Oct 4, 2023

View reviewed changes

Add cargo tests CI, add dummy test inside tests folder

a1de3ec

niklasdewally requested changes Oct 4, 2023

View reviewed changes

niklasdewally approved these changes Oct 4, 2023

View reviewed changes

ozgurakgun merged commit cda01a7 into conjure-cp:main Oct 4, 2023
3 checks passed

ozgurakgun mentioned this pull request Oct 4, 2023

Kissat bindings #7

Merged

Kieranoski702 mentioned this pull request Jan 19, 2024

Continue Chuffed Rust Bindings Implementation conjure-cp/VIP#5

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chuffed: initial commit #6

Chuffed: initial commit #6

Kieranoski702 commented Oct 2, 2023 •

edited

Loading

yigityazicilar commented Oct 2, 2023

niklasdewally commented Oct 2, 2023

yigityazicilar commented Oct 2, 2023

yigityazicilar commented Oct 2, 2023

yigityazicilar Oct 2, 2023

yigityazicilar Oct 3, 2023

Kieranoski702 Oct 3, 2023

yigityazicilar Oct 3, 2023

Kieranoski702 Oct 3, 2023

yigityazicilar Oct 3, 2023

Kieranoski702 Oct 3, 2023

ozgurakgun commented Oct 3, 2023

Kieranoski702 commented Oct 3, 2023

niklasdewally commented Oct 3, 2023 •

edited

Loading

yigityazicilar commented Oct 3, 2023

ChrisJefferson commented Oct 4, 2023

niklasdewally commented Oct 4, 2023

This comment was marked as duplicate.

niklasdewally left a comment

niklasdewally left a comment •

edited

Loading

Chuffed: initial commit #6

Chuffed: initial commit #6

Conversation

Kieranoski702 commented Oct 2, 2023 • edited Loading

yigityazicilar commented Oct 2, 2023

niklasdewally commented Oct 2, 2023

yigityazicilar commented Oct 2, 2023

yigityazicilar commented Oct 2, 2023

yigityazicilar Oct 2, 2023

Choose a reason for hiding this comment

yigityazicilar Oct 3, 2023

Choose a reason for hiding this comment

Kieranoski702 Oct 3, 2023

Choose a reason for hiding this comment

yigityazicilar Oct 3, 2023

Choose a reason for hiding this comment

Kieranoski702 Oct 3, 2023

Choose a reason for hiding this comment

yigityazicilar Oct 3, 2023

Choose a reason for hiding this comment

Kieranoski702 Oct 3, 2023

Choose a reason for hiding this comment

ozgurakgun commented Oct 3, 2023

Kieranoski702 commented Oct 3, 2023

niklasdewally commented Oct 3, 2023 • edited Loading

yigityazicilar commented Oct 3, 2023

ChrisJefferson commented Oct 4, 2023

niklasdewally commented Oct 4, 2023

This comment was marked as duplicate.

niklasdewally left a comment

Choose a reason for hiding this comment

niklasdewally left a comment • edited Loading

Choose a reason for hiding this comment

Kieranoski702 commented Oct 2, 2023 •

edited

Loading

niklasdewally commented Oct 3, 2023 •

edited

Loading

niklasdewally left a comment •

edited

Loading