JuliaInterop / CxxWrap.jl

Package to make C++ libraries available in Julia
Other
418 stars 67 forks source link

The appropriate way to use CxxWrap in julia 1.0 with a cmake project. #115

Closed TransGirlCodes closed 5 years ago

TransGirlCodes commented 6 years ago

I'm part of a C++ project here built using cmake.

The project has a folder called "interfaces" at the top of the directory tree which includes source files for interfaces for other languages.

So I wanted to have the C++ file, containing the C++ code side of CxxWrap here. Then I wanted to set the project so as if the argument JlCxx_DIR is passed to cmake e.g.: -DJlCxx_DIR=/Users/bward/.julia/packages/CxxWrap/jLyxA/deps/usr, then the project will build the C++ part of the CxxWrap wrapper.

Then in the future I can define a Julia package which builds the project with this option, and uses the julia side of CxxWrap to complete the wrapper.

I've done a minimal example on the branch juliawrap.

I achieved this by the following cmake code:

IF (JlCxx_DIR)
    list(APPEND CMAKE_PREFIX_PATH ${JlCxx_DIR}/lib/cmake/JlCxx)
    message("Building julia (The superior language TM) wrapper")
    message("Using cmake prefix path: ${JlCxx_DIR}/lib/cmake/JlCxx")
    message("Using jlcxx include: ${JlCxx_DIR}/include")
    include_directories(${JlCxx_DIR}/include)
    find_package(JlCxx REQUIRED)
    IF (JlCxx_FOUND)
        include_directories(${Julia_INCLUDE_DIRS})
        add_library(bsg_julia SHARED interfaces/JuliaBSG.cpp)
        target_link_libraries(bsg_julia bsg)
        target_link_libraries(bsg_julia JlCxx::cxxwrap_julia)
    ENDIF()
ENDIF()

Then to cmake I have to provide the following arguments: -DJulia_ROOT=/Users/bward/github/julia/usr -DJlCxx_DIR=/Users/bward/.julia/packages/CxxWrap/jLyxA/deps/usr

Is this the correct or best way to go about this? I'm suspecting not as it involves specifying exact paths which would be invalid for anyone else's machine or julia install (e.g. CxxWrap/jLyxA). How can I do this better?

barche commented 6 years ago

This is more or less the idea, but it should normally only be necessary to specify JlCxx_DIR, then the Julia used to build libcxxwrap-julia should be picked up automatically. Manually adding the Julia includes should also not be needed. This procedure should be followed by anyone building your package from source. For binaries, I recommend using BinaryBuilder, examples are libcxxwrap-julia itself or LCIO: https://github.com/jstrube/LCIOWrapBuilder

TransGirlCodes commented 5 years ago

Hey @barche

I've edited my cmake for use with binary builder, which I've set to download the binary dep of JlCxx and the 'fake' BinaryBuilder build of Julia. So the cmake currently looks like so:

#IF (JlCxx_DIR)
IF (BUILD_JULIA_INTERFACE)
    #list(APPEND CMAKE_PREFIX_PATH ${JlCxx_DIR}/lib/cmake/JlCxx)
    #message("Using cmake prefix path: ${JlCxx_DIR}/lib/cmake/JlCxx")
    #message("Using jlcxx include: ${JlCxx_DIR}/include")
    #include_directories(${JlCxx_DIR}/include)
    find_package(JlCxx REQUIRED)
    #IF (JlCxx_FOUND)
        include_directories(${Julia_INCLUDE_DIRS})
        #get_property(dirs DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY INCLUDE_DIRECTORIES)
        #foreach(dir ${dirs})
        #    message(STATUS "dir='${dir}'")
        #endforeach()

        add_library(bsg_julia SHARED interfaces/JuliaBSG.cpp)
        target_link_libraries(bsg_julia bsg)
        target_link_libraries(bsg_julia JlCxx::cxxwrap_julia)
    #ENDIF()
ENDIF()

But during the cmake run: cmake -DCMAKE_INSTALL_PREFIX=$prefix -DCMAKE_TOOLCHAIN_FILE=/opt/$target/$target.toolchain -DBUILD_JULIA_INTERFACE=on

In the BinaryBuilder Wizard I see the following occur:

-- Found Julia executable: /workspace/destdir/bin/julia
-- Julia_VERSION_STRING: 1.0.0
-- Julia_INCLUDE_DIRS:   /workspace/destdir/include/julia
-- Julia_LIBRARY_DIR:    /workspace/destdir/lib
-- Julia_LIBRARY:        /workspace/destdir/lib/libjulia.so
-- JULIA_HOME:           /workspace/destdir/bin
-- Julia_LLVM_VERSION:   v6.0.0
-- Julia_WORD_SIZE:      64
-- Found Julia: /workspace/destdir/lib/libjulia.so (found version "1.0.0") 
-- Configuring done
CMake Error in CMakeLists.txt:
  Imported target "JlCxx::cxxwrap_julia" includes non-existent path

    "/opt/x86_64-linux-gnu/x86_64-linux-gnu/sys-root/usr/local/include"

  in its INTERFACE_INCLUDE_DIRECTORIES.  Possible reasons include:

  * The path was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and references files it does not
  provide.

CMake Error in CMakeLists.txt:
  Imported target "JlCxx::cxxwrap_julia" includes non-existent path

    "/opt/x86_64-linux-gnu/x86_64-linux-gnu/sys-root/usr/local/include"

  in its INTERFACE_INCLUDE_DIRECTORIES.  Possible reasons include:

  * The path was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and references files it does not
  provide.

-- Generating done
-- Build files have been written to: /workspace/srcdir/bsg

Do you know why this might be?

barche commented 5 years ago

On the newest BinaryBuilder it seems to be necessary to define CMAKE_FIND_ROOT_PATH. There is now a test library in the source tree of lincxxwrap-julia, this should always provide an up-to-date template to build against it: https://github.com/JuliaInterop/libcxxwrap-julia/blob/master/testlib-builder/build_tarballs.jl#L10

TransGirlCodes commented 5 years ago

Presumably my build script does not have to worry about this sort of stuff:

# This transforms the dependency script to use the binaries generated in the same Travis run
@show ws_root = dirname(dirname(dirname(dirname(@__FILE__))))
@show prod_dir = joinpath(ws_root, "products")
@show generated_script_name = joinpath(prod_dir,"build_libcxxwrap-julia-1.0.$(version_number).jl")
local_dep_script = ""
open(generated_script_name, "r") do generated_script
    while !eof(generated_script)
        l = readline(generated_script; keep=true)
        if startswith(l, "bin_prefix = \"https://github.com")
            l = "bin_prefix = \"$prod_dir\"\n"
        end
        global local_dep_script *= l
    end
end

As I'll be getting libcxxwrap-julia as a binary dependency.

I suppose as well as the BinaryBuilder script, I will also have to change my cmake to be more similar to the example cmake:

IF (BUILD_JULIA_INTERFACE)
    message("Building Julia wrapper")
    #message("Using cmake prefix path: ${JlCxx_DIR}/lib/cmake/JlCxx")
    #message("Using jlcxx include: ${JlCxx_DIR}/include")
    #include_directories(${JlCxx_DIR}/include)
    find_package(JlCxx REQUIRED)
    get_target_property(JlCxx_location JlCxx::cxxwrap_julia LOCATION)
    get_filename_component(JlCxx_location ${JlCxx_location} DIRECTORY)
    set(CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/lib;${JlCxx_location}")
    message(STATUS "Founc JlCxx at ${JlCxx_location}")
    #IF (JlCxx_FOUND)
        #include_directories(${Julia_INCLUDE_DIRS})
        #get_property(dirs DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY INCLUDE_DIRECTORIES)
        #foreach(dir ${dirs})
        #    message(STATUS "dir='${dir}'")
        #endforeach()

        add_library(bsgjl SHARED interfaces/JuliaBSG.cpp)
        target_link_libraries(bsgjl bsg)
        target_link_libraries(bsgjl JlCxx::cxxwrap_julia)
    #ENDIF()
    install(TARGETS bsgjl LIBRARY DESTINATION lib ARCHIVE DESTINATION lib RUNTIME DESTINATION lib)
ENDIF()
barche commented 5 years ago

Yes, the script transformation is needed only because I need to use the binaries from the running build. The CMake looks OK at first glance, but the proof of the pudding is in the eating I guess ;)

TransGirlCodes commented 5 years ago

I'm getting the following when I finally try to wrap the library:

(v1.0) pkg> activate .

julia> using BSG
[ Info: Precompiling BSG [3c37b7a2-d3b5-11e8-3e9e-e917918d9941]
Assertion failed: (type_pointer() != nullptr), function julia_reference_type, file /workspace/destdir/include/jlcxx/type_conversion.hpp, line 330.

signal (6): Abort trap: 6
in expression starting at /Users/bward/Desktop/BSG/src/BSG.jl:3
__pthread_kill at /usr/lib/system/libsystem_kernel.dylib (unknown line)
Allocations: 2328591 (Pool: 2328062; Big: 529); GC: 4
ERROR: Failed to precompile BSG [3c37b7a2-d3b5-11e8-3e9e-e917918d9941] to /Users/bward/.julia/compiled/v1.0/BSG/abDuO.ji.
Stacktrace:
 [1] macro expansion at ./logging.jl:313 [inlined]
 [2] compilecache(::Base.PkgId, ::String) at ./loading.jl:1187
 [3] macro expansion at ./logging.jl:311 [inlined]
 [4] _require(::Base.PkgId) at ./loading.jl:944
 [5] require(::Base.PkgId) at ./loading.jl:855
 [6] macro expansion at ./logging.jl:311 [inlined]
 [7] require(::Module, ::Symbol) at ./loading.jl:837
barche commented 5 years ago

This indicates that an argument type or return type of a function is not wrapped, i.e. add_type is not called on the type before wrapping a function using it.

TransGirlCodes commented 5 years ago

@barche Ohhh after some playing to figure out what is not wrapped... the issues are class methods that accept vectors of some thing, and it makes sense Vectors being templates are not wrapped automatically. Can I specify specific vector classes for my wrapper? vector for example?

barche commented 5 years ago

Yes, it should be possible to just add_type<std::vector< sgNodeStatus_t>>("sgNodeStatusVector"). If there are many types, it is better to wrap the vector as a template type (see the parametric example). You may also need to add extra wrapper functions that take an ArrayRef if you want to pass an array constructed on the Julia side.

SylvainCorlay commented 5 years ago

@barche @benjward you can also look at the xtensor-julia-cookiecutter which generates a template project for cxxwrap and xtensor. Very little of it is specific to xtensor and you can see it as a means to generate all the packaging boilerplate for a project using CxxWrap.

SylvainCorlay commented 5 years ago

https://github.com/QuantStack/xtensor-julia-cookiecutter

TransGirlCodes commented 5 years ago

@barche Did some more playing, it looks like class methods that return vectors like this one:

std::vector<Link> get_fw_links( sgNodeID_t n) const ;

Where sgNodeID_t is just an alias to int64.

Compile and get wrapped ok just by adding the method: .method("get_fw_links", &SequenceGraph::get_fw_links)

But a method with more involved input like this one:

void write_to_gfa(std::string filename, const std::vector<std::vector<Link>> &arg_links={},
                          const std::vector<sgNodeID_t> &selected_nodes={}, const std::vector<sgNodeID_t> &mark_red={},
                          const std::vector<double> &depths={});

When wrapped as .method("write_to_gfa", &SequenceGraph::write_to_gfa).

Will cause the error when I try to using BSG from my julia.

So it seems outputting vectors is fine, but using them as input parameters is the issue.

Maybe I should create a lambda that stands in for "write_to_gfa" that accepts julia arrays of sgNodeID_t and then just constructs a std::vector from said array as an intermediary step?

barche commented 5 years ago

Yes, every one of the vector types in the argument list needs to be wrapped before wrapping that function. The best way forward depends on how these arguments are generated: does the library provide functions that return these vectors, or are users meant to create them from scratch? In the former case, it is better to just wrap the vectors and let the C++ side manage them. If the user has to build the vectors, it would be nicest for a Julia wrapper if they accept Julia arrays, so then a wrapper function that takes jlcxx::ArrayRef is the way to go and possibly not all of the vector types need to be wrapped.

TransGirlCodes commented 5 years ago

@barche

I've tried to use ArrayRef to wrap a class method where the return type is a vector:

.method("get_fw_nodes", [](SequenceGraph& sg, sgNodeID_t n) {
                std::vector<sgNodeID_t> fw_nodes = sg.get_fw_nodes(n);
                for(const auto i:fw_nodes) {std::cout << i << std::endl;}
                return jlcxx::ArrayRef<sgNodeID_t,1>(&(fw_nodes[0]), fw_nodes.size());
            })

This seems to make sense, use a lambda, to use the class method, get the vector, and initialize an ArrayRef using the vector.

The for loop is in there to print the vector vals so I can check the output is correct.

When I use it from Julia, I get odd outputs:

julia> using BSG
[ Info: Recompiling stale cache file /Users/bward/.julia/compiled/v1.0/BSG/abDuO.ji for BSG [3c37b7a2-d3b5-11e8-3e9e-e917918d9941]
WARNING: Method definition (::Type{BSG.KmerIDX})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.KmerIDX128})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.graphStrandPos})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.nodeVisitor})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.Link})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.SequenceGraph})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.WorkSpace})() in module BSG overwritten.

julia> ws = BSG.WorkSpace()
BSG.WorkSpaceAllocated(Ptr{Nothing} @0x00007fabee5328f0)

julia> BSG.load_from_disk(ws, "/Users/bward/Desktop/ecoli_workspace/workspace_kci_ecoli.bsgws", false)

2018-11-14 17:01:14: Loaded graph with 210 nodes
2018-11-14 17:01:14: KCI Mean coverage for unique kmers:   84.2364
2018-11-14 17:01:14: KCI Median coverage for unique kmers: 84
2018-11-14 17:01:14: KCI Mode coverage for unique kmers:   80

julia> sg = BSG.getGraph(ws)
BSG.SequenceGraphRef(Ptr{Nothing} @0x00007fabee532908)

julia> BSG.get_fw_nodes(sg, 5)
48
58
2-element Array{Int64,1}:
      4778135232
 140376415624304

See in my array I should have the two values 48 and 58, but instead I get two random Int64's. (On the C++ side sgNodeID_t is a typedef alias for 64 bit integers: typedef int64_t sgNodeID_t;. What have I done wrong?

TransGirlCodes commented 5 years ago

I've revised it to:

.method("get_fw_nodes", [](SequenceGraph& sg, sgNodeID_t n) {
                std::vector<sgNodeID_t> fw_nodes = sg.get_fw_nodes(n);
                for(const auto i:fw_nodes) {std::cout << i << std::endl;}
                return jlcxx::ArrayRef<sgNodeID_t>(fw_nodes.data(), fw_nodes.size());
            })

To use the new vector::data method, but still no joy:

(v1.0) pkg> activate .

julia> using BSG
[ Info: Recompiling stale cache file /Users/bward/.julia/compiled/v1.0/BSG/abDuO.ji for BSG [3c37b7a2-d3b5-11e8-3e9e-e917918d9941]
WARNING: Method definition (::Type{BSG.KmerIDX})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.KmerIDX128})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.graphStrandPos})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.nodeVisitor})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.Link})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.SequenceGraph})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.WorkSpace})() in module BSG overwritten.

julia> ws = BSG.WorkSpace()
BSG.WorkSpaceAllocated(Ptr{Nothing} @0x00007f866bf67ce0)

julia> BSG.load_from_disk(ws, "/Users/bward/Desktop/ecoli_workspace/workspace_kci_ecoli.bsgws", false)

2018-11-14 22:50:33: Loaded graph with 210 nodes
2018-11-14 22:50:33: KCI Mean coverage for unique kmers:   84.2364
2018-11-14 22:50:33: KCI Median coverage for unique kmers: 84
2018-11-14 22:50:33: KCI Mode coverage for unique kmers:   80

julia> sg = BSG.getGraph(ws)
BSG.SequenceGraphRef(Ptr{Nothing} @0x00007f866bf67cf8)

julia> BSG.get_fw_nodes(sg, 5)
48
58
2-element Array{Int64,1}:
      4691543072
 140215333516608
TransGirlCodes commented 5 years ago

@barche It occured to me, hey maybe since the ArrayRef is created C++ side, its memory is not owned by julia, I thought setting julia_owned = true in the constructor would help that, but it did not:

.method("get_fw_nodes", [](SequenceGraph& sg, sgNodeID_t n) {
                std::vector<sgNodeID_t> fw_nodes = sg.get_fw_nodes(n);
                for(const auto i:fw_nodes) {std::cout << i << std::endl;}
                return jlcxx::ArrayRef<sgNodeID_t>(true, fw_nodes.data(), fw_nodes.size());
            })
(v1.0) pkg> activate .

julia> using BSG
[ Info: Recompiling stale cache file /Users/bward/.julia/compiled/v1.0/BSG/abDuO.ji for BSG [3c37b7a2-d3b5-11e8-3e9e-e917918d9941]
WARNING: Method definition (::Type{BSG.KmerIDX})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.KmerIDX128})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.graphStrandPos})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.nodeVisitor})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.Link})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.SequenceGraph})() in module BSG overwritten.
WARNING: Method definition (::Type{BSG.WorkSpace})() in module BSG overwritten.

julia> ws = BSG.WorkSpace()
BSG.WorkSpaceAllocated(Ptr{Nothing} @0x00007fabacee34a0)

julia> BSG.load_from_disk(ws, "/Users/bward/Desktop/ecoli_workspace/workspace_kci_ecoli.bsgws", false)

2018-11-14 23:25:23: Loaded graph with 210 nodes
2018-11-14 23:25:23: KCI Mean coverage for unique kmers:   84.2364
2018-11-14 23:25:23: KCI Median coverage for unique kmers: 84
2018-11-14 23:25:23: KCI Mode coverage for unique kmers:   80

julia> sg = BSG.getGraph(ws)
BSG.SequenceGraphRef(Ptr{Nothing} @0x00007fabacee34b8)

julia> BSG.get_fw_nodes(sg, 5)
48
58
2-element Array{Int64,1}:
      4533903584
 140375315955072
TransGirlCodes commented 5 years ago

I'll close this an open a new issue as my problem is unrelated to the origional.