Use Cint to match C++'s int type, not Julia's Int.#2283
Use Cint to match C++'s int type, not Julia's Int.#2283favre49 merged 6 commits intomlpack:masterfrom
Conversation
|
Hi @rcurtin I want to ask that can we can Cdouble in place of Float64. |
|
@Yashwants19 correct me if I'm wrong, but it seems to me on https://docs.julialang.org/en/v1/manual/calling-c-and-fortran-code/#man-bits-types-1 like |
|
https://github.com/mlpack/mlpack/blob/f482383a8bb0f915829510879afa604cc514cabd/src/mlpack/bindings/julia/mlpack/cli.jl.in#L213 |
Sorry I think so I had overlooked this part. :) |
| -DARMADILLO_LIBRARY="..\armadillo-8.400.0\Release\armadillo.lib" ` | ||
| -DBOOST_INCLUDEDIR=$(Agent.ToolsDirectory)\boost.1.60.0.0\lib\native\include ` | ||
| -DBOOST_LIBRARYDIR=$(Agent.ToolsDirectory)\boost_libs ` | ||
| -DBUILD_JULIA_BINDINGS=OFF ` |
There was a problem hiding this comment.
Hm, that is strange looks like we are still trying to run the julia bindings tests.
There was a problem hiding this comment.
Yeah, the debugging exposed a bug in the CMake configuration. 😄 I think 3f07bcf fixes it.
There was a problem hiding this comment.
Nice, I guess with #2278 we should see more green builds.
There was a problem hiding this comment.
Here's hoping! It's been a really bad week for our build systems and I think a good part of that is my fault. 😄
Nice catch! Thank you. Fixed in 4bb869a. |
|
I will wait until Azure builds this time before merging. 😄 |
|
Hm, let's see if we can bring the azure build back. |
|
Manually started the build: https://dev.azure.com/mlpack/mlpack/_build/results?buildId=998&view=results not sure why the last commit hasn't triggered the build. |
|
Looks like all the builds passed (besides MacOS) :). I'll go ahead and merge it |
I mistakenly thought that Julia's
Inttype always matched C/C++'sinttype; this is incorrect---Julia'sInttype instead matchessize_t. Therefore, we have to use theCinttype in Julia. This PR fixes that and all associated handling, and should fix the currently-failing Julia build.