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

BinDeps integration #12

Merged
merged 1 commit into from
Jun 1, 2014
Merged

BinDeps integration #12

merged 1 commit into from
Jun 1, 2014

Conversation

tkelman
Copy link
Contributor

@tkelman tkelman commented May 29, 2014

Builds from source on Unix, downloads binaries on Windows, basic functionality appears to work.
Haven't tested this on Mac, could eventually use a Homebrew formula and supply bottles for users who don't have compilers installed.

Fixes part of #6, would be a good idea to turn the examples into tests, set up Travis, etc.

The Windows binaries were built from Cygwin as follows

../configure --enable-shared --host=i686-w64-mingw32 --prefix=$PWD/usr
make LDFLAGS="-no-undefined -avoid-version"
make LDFLAGS="-no-undefined -avoid-version" install

for 32 bit, or replacing i686 with x86_64 for 64 bit.

@acroy
Copy link
Contributor

acroy commented May 29, 2014

Very nice! Seems to work for me on OSX & Julia 0.3-pre ... I will test it tomorrow on Linux. Do you know if there is some "standard" regarding binaries on Mac? Using a Homebrew formula sounds good though. Another question concerns the push! to LOAD_PATH: is that usually done like this in other packages or is the user supposed to do it manually?

@tkelman
Copy link
Contributor Author

tkelman commented May 29, 2014

Thanks for checking, and catching my typo on the mailing list (I should take a nap).

Regarding standard for binaries on Mac, see https://github.com/JuliaLang/Homebrew.jl#why-package-authors-should-use-homebrewjl - basically a Homebrew formula serves as a recipe for building binary bottles, which Elliot Saba has been nice enough to create and host. So BinDeps doesn't use the system-wide Homebrew installation, rather it uses the Julia-specific Homebrew package which is designed to primarily download binaries. That way users can get working Mac binaries without needing compilers installed, once it's all set up (unlike standard Homebrew which relies on Xcode command line tools).

I think with more sophisticated use of BinDeps I could get away without pushing to LOAD_PATH. If the @BinDeps.install line is followed with mappings like the following [:cvode => :cvode] then it creates a deps/deps.jl file that you can then include before the ccalls, which defines paths to all the libraries. That seems to be how most libraries handle this (for examples see https://github.com/staticfloat/Nettle.jl/blob/master/deps/build.jl or several packages in JuliaOpt), although I'm having trouble getting the important @checked_lib lines to show up when I try that method here. HDF5 does things more like I did here, at least on Windows: https://github.com/timholy/HDF5.jl/blob/master/src/plain.jl#L33

@acroy
Copy link
Contributor

acroy commented May 30, 2014

Not very surprisingly it also works on my Linux box (RHEL). I looked a bit into the INSTALL_NOTES and noticed two options --with-lapack and --with-blas, which can be used to specify the location of the LAPACK and BLAS libraries. Ideally we want to use the same libs as Julia I guess?

Regarding Homebrew.jl, I think this would be nice to have for Sundials as well, but it could probably also be added later?

A problem with explicitly pushing to LOAD_PATH in the package might arise when someone wants to use his/her own installation of the libraries? I guess that BinDeps can handle this and puts the right paths in deps/deps.jl?

@tkelman
Copy link
Contributor Author

tkelman commented May 30, 2014

Re: Lapack and Blas, I'm not familiar enough with Sundials internals to say for sure. I believe the configure script searches for -lblas and -llapack, so if you have reference Blas/Lapack installed through your package manager it should use those.

The majority of code I've seen that uses Blas and Lapack expects integers to always be 32 bit (LP64 on 64 bit systems), however 64-bit Julia builds its Blas and Lapack libraries with 64 bit integers (ILP64) by default. This can cause segfaults or unexpected behavior if you try to use a library built for one with code that expects the other. There's an issue open in Julia about this and it's shown up in a few packages so far, no great solution yet.

I'm not so sure what happens if you already have your own installation of the library in a system path, which I guess would've been the only way to use this package prior to this PR. Would the package get rebuilt redundantly? It looks like there are Ubuntu and Debian packages for Sundials here http://packages.ubuntu.com/precise/libsundials-serial, we can add a few lines here to let BinDeps know that apt-get (and maybe yum on Fedora?) is a possible binary provider.

@acroy
Copy link
Contributor

acroy commented May 30, 2014

I guess that the package is not (re)built if BinDeps can find it in a system path, anything else would be strange. The problem is that you do not take that possibility into account when pushing to DL_LOAD_PATH. BinDeps probably does that when it is creating deps/deps.jl though?

@tkelman
Copy link
Contributor Author

tkelman commented May 30, 2014

Yeah, I think the created deps/deps.jl ends up listing the system path, or no path at all (just the lib name). If someone can help me figure out how to tweak this to create that file including the @checked_lib line I'll happily switch this over to use that method.

@ihnorton
Copy link

If someone can help me figure out how to tweak this to create that file including the @checked_lib line I'll happily switch this over to use that method.

Pass a Dictionary to BinDeps.install with mapping the library name specified in the deps = [...] map, to the name that the library expects internally. See e.g.:

https://github.com/JuliaLang/Cairo.jl/blob/master/deps/build.jl#L129-L131 (I don't remember why only four of them are specified here)

https://github.com/staticfloat/Nettle.jl/blob/master/deps/build.jl

@acroy
Copy link
Contributor

acroy commented May 30, 2014

Seems you have to do it like that

cvode = library_dependency("cvode", aliases = ["libsundials_cvode"])
[/snip/]
@BinDeps.install [:cvode => :cvode]

which gives in deps/deps.jl

@checked_lib cvode "/Users/acroy/.julia/v0.3/Sundials/deps/usr/lib/libsundials_cvode"

Do we have to do this for each library (nvecserial, cvode, cvodes, ida, idas, kinsol)?

@tkelman
Copy link
Contributor Author

tkelman commented May 30, 2014

Aha, was missing this "cvode", aliases = ["libsundials_cvode"] part - will give it another go. I think we need all 6 libs, yes.

@acroy
Copy link
Contributor

acroy commented May 30, 2014

I guess

@BinDeps.install [:libsundials_cvode => :libsundials_cvode]

would also work with your previous call to library_dependency?

@powerdistribution
Copy link
Contributor

Awesome! Thanks for working on this. I'm surprised how short the code is.

@ihnorton
Copy link

BinDeps can be mystifying, but it's also really powerful once you get the hang of it.

@tkelman
Copy link
Contributor Author

tkelman commented May 30, 2014

Okay, here's a version using the deps.jl file instead of DL_LOAD_PATH. Seems to work on Linux (RHEL) and on 64 bit Windows. How's it work on Mac or on a system where Sundials is already there from the package manager?

Edit: there is some trouble with the 32 bit Windows binary (access violation in N_VScale_Serial), looks like I need to rebuild it.

@acroy
Copy link
Contributor

acroy commented May 30, 2014

Also works on my Mac and seems to do the right thing when Sundials was already installed (at least it creates deps.jl with the correct path). I would say this looks good! I am not sure if we really want apt/yum providers - personally I prefer it the way it is, but others might want them?

@tkelman
Copy link
Contributor Author

tkelman commented May 30, 2014

32-bit Windows may need some more serious debugging. Here's what I'm seeing when I use a debug build of Sundials. Haven't gotten much useful info out of gdb yet. The C examples from Sundials run fine. Need to download a newer 32-bit Julia nightly and check if the same thing happens.

  | | |_| | | | (_| |  |  Version 0.3.0-prerelease+2703 (2014-04-22 18:57 UTC)
 _/ |\__'_|_|_|\__'_|  |  Commit 942ae42* (37 days old master)
|__/                   |  i686-w64-mingw32

julia> function f(t, y, ydot)
           ydot[1] = -0.04*y[1] + 1.0e4*y[2]*y[3]
           ydot[3] = 3.0e7*y[2]*y[2]
           ydot[2] = -ydot[1] - ydot[3]
       end
f (generic function with 1 method)

julia> t = [0.0, 4 * logspace(-1., 7., 9)]
10-element Array{Float64,1}:
      0.0
      0.4
      4.0
     40.0
    400.0
   4000.0
  40000.0
 400000.0
      4.0e6
      4.0e7

julia> using Sundials

julia> res = Sundials.ode(f, [1.0, 0.0, 0.0], t)
Please submit a bug report with steps to reproduce this fault, and any error mes
sages that follow (in their entirety). Thanks.
Exception: EXCEPTION_ACCESS_VIOLATION at 0x6ebc2d09 -- ??? at ???: offset 6ebc2d
09
N_VMinQuotient_Serial at ???: offset 6ebc2d09
N_VScale_Serial at ???: offset 6ebc22fe
N_VScale at ???: offset 6339814f
CVodeInit at ???: offset 63381a70
utf8proc_NFKC at ???: offset 6f2e1fa9
jl_apply_generic at ???: offset 6ec10cd6
jl_interpret_toplevel_expr at ???: offset 6ec43f12
jl_interpret_toplevel_thunk_with at ???: offset 6ec43270
jl_interpret_toplevel_thunk_with at ???: offset 6ec434d7
jl_interpret_toplevel_expr_in at ???: offset 6ec44406
jl_interpret_toplevel_thunk_with at ???: offset 6ec42e51
jl_interpret_toplevel_thunk at ???: offset 6ec44731
jl_eval_with_compiler_p at ???: offset 6ec53b2d
jl_f_top_eval at ???: offset 6ec16ad0
jl_apply_generic at ???: offset 6ec10cd6
jl_apply_generic at ???: offset 6ec10cd6
jl_f_tupleref at ???: offset 6ec161b7
jl_generate_fptr at ???: offset 6ec1f15c
jl_f_task at ???: offset 6ec4aab8
julia_trampoline at ???: offset 6ec4985c
tgamma at ???: offset 41c028
??? at ???: offset 4013de
BaseThreadInitThunk at ???: offset 7685338a
RtlInitializeExceptionChain at ???: offset 77519f72
RtlInitializeExceptionChain at ???: offset 77519f45

@acroy
Copy link
Contributor

acroy commented May 30, 2014

:-( What happens if you do the call to CVodeInit manually?

y0 = [1.0, 0.0, 0.0]
mem = Sundials.CVodeCreate(Sundials.CV_BDF, Sundials.CV_NEWTON)
flag = Sundials.CVodeInit(mem, cfunction(Sundials.odefun, Int32, (Sundials.realtype, Sundials.N_Vector, Sundials.N_Vector, Function)), t[1], Sundials.nvector(y0))

@acroy acroy mentioned this pull request May 30, 2014
@tkelman
Copy link
Contributor Author

tkelman commented May 31, 2014

Same crash. I have a theory, most or all instances of Int64 may need to be replaced by Clong. Will try it out in a minute. I'm a little surprised that Win64 worked though. Anyone have access to a 32 bit Linux machine to test if the same problem happens there?

@acroy
Copy link
Contributor

acroy commented May 31, 2014

Thanks for pinning this down. But it is really weird, as you say one would have expected that it doesn't work on Win64 in the first place!? Could this depend on the compilers used for building Julia and Sundials, respectively?

@tkelman
Copy link
Contributor Author

tkelman commented Jun 1, 2014

Might depend on compilers, might also be something to do with bit alignment. I was just making sure a simple test case ran without erroring or crashing, so it may have been giving incorrect results on Win64 - I should've cross-checked the numbers more carefully.

* add BinDeps to require

* first cut at linux deps/build.jl

* use SimpleBuild (make install wasn't running with the Autotools BuildProcess)

* add windows binary download

* use deps.jl file instead of DL_LOAD_PATH
@tkelman
Copy link
Contributor Author

tkelman commented Jun 1, 2014

Squashed and rebased (to test incorporating the changes from #13) - this now runs without crashing on Linux 64, Windows 32, and Windows 64. Any other platforms where Clong == Int64 should have no change relative to yesterday.

The numerical results from cvode_Roberts_simplified.jl are slightly different on Windows 32. I'm not sure where the discrepancy might be coming from.

@acroy
Copy link
Contributor

acroy commented Jun 1, 2014

Bitalignment sounds plausible. Would be interesting to know if it ever worked on 32bit Linux. But this shouldn't prevent us merging your PR given the overall benefits.

Anyways, it still works on the Mac. Just for reference, cvode_Roberts_simplified.jl gives for me

10x3 Array{Float64,2}:
 1.0          0.0          0.0      
 0.98517      3.38642e-5   0.0147962
 0.905459     2.23972e-5   0.0945187
 0.715738     9.18273e-6   0.284253 
 0.450404     3.22352e-6   0.549592 
 0.183051     8.93306e-7   0.816948 
 0.0389733    1.62129e-7   0.961027 
 0.00493432   1.98334e-8   0.995066 
 0.000518888  2.07669e-9   0.999481 
 5.15832e-5   2.06342e-10  0.999948

How big is the difference on Win32?

@tkelman
Copy link
Contributor Author

tkelman commented Jun 1, 2014

Not very

10x3 Array{Float64,2}:
 1.0         0.0          0.0
 0.98517     3.38642e-5   0.0147962
 0.905459    2.23972e-5   0.0945187
 0.715738    9.18272e-6   0.284253
 0.450367    3.2216e-6    0.54963
 0.183224    8.94363e-7   0.816775
 0.0389795   1.62161e-7   0.96102
 0.00494222  1.98663e-8   0.995058
 0.00051539  2.06261e-9   0.999485
 6.05883e-5  2.42353e-10  0.999939

@acroy
Copy link
Contributor

acroy commented Jun 1, 2014

If you happen to have them compiled, you can compare to the output of the example (cvRoberts_dns) in sundials-2.5.0/examples/cvode/serial/. I suspect that the discrepancy has nothing to do with Julia.

If there are no objections (and no one else beats me to it), I am going to merge it ...

@tkelman
Copy link
Contributor Author

tkelman commented Jun 1, 2014

Slight differences, but neither matches the results from Julia exactly, or the cvRoberts_dns.out file included with the source. At least Win64 C example results match Linux 64 C example results, that's encouraging.

Win64
$ ./cvRoberts_dns

3-species kinetics problem

At t = 2.6391e-001      y = 9.899653e-001   3.470564e-005   1.000000e-002
    rootsfound[] =   0   1
At t = 4.0000e-001      y = 9.851641e-001   3.386242e-005   1.480205e-002
At t = 4.0000e+000      y = 9.055097e-001   2.240338e-005   9.446793e-002
At t = 4.0000e+001      y = 7.158017e-001   9.185037e-006   2.841892e-001
At t = 4.0000e+002      y = 4.505360e-001   3.223271e-006   5.494608e-001
At t = 4.0000e+003      y = 1.832299e-001   8.944378e-007   8.167692e-001
At t = 4.0000e+004      y = 3.898902e-002   1.622006e-007   9.610108e-001
At t = 4.0000e+005      y = 4.936383e-003   1.984224e-008   9.950636e-001
At t = 4.0000e+006      y = 5.168093e-004   2.068293e-009   9.994832e-001
At t = 2.0790e+007      y = 1.000000e-004   4.000397e-010   9.999000e-001
    rootsfound[] =  -1   0
At t = 4.0000e+007      y = 5.202440e-005   2.081083e-010   9.999480e-001
At t = 4.0000e+008      y = 5.201061e-006   2.080435e-011   9.999948e-001
At t = 4.0000e+009      y = 5.258603e-007   2.103442e-012   9.999995e-001
At t = 4.0000e+010      y = 6.934511e-008   2.773804e-013   9.999999e-001

Final Statistics:
nst = 542    nfe  = 755    nsetups = 107    nfeLS = 0      nje = 11
nni = 751    ncfn = 0      netf = 22     nge = 570


Win32
$ ./cvRoberts_dns

3-species kinetics problem

At t = 2.6391e-001      y = 9.899653e-001   3.470564e-005   1.000000e-002
    rootsfound[] =   0   1
At t = 4.0000e-001      y = 9.851641e-001   3.386242e-005   1.480205e-002
At t = 4.0000e+000      y = 9.055097e-001   2.240338e-005   9.446793e-002
At t = 4.0000e+001      y = 7.157952e-001   9.183486e-006   2.841956e-001
At t = 4.0000e+002      y = 4.505420e-001   3.222962e-006   5.494548e-001
At t = 4.0000e+003      y = 1.831868e-001   8.941136e-007   8.168123e-001
At t = 4.0000e+004      y = 3.898774e-002   1.621958e-007   9.610121e-001
At t = 4.0000e+005      y = 4.936986e-003   1.984473e-008   9.950630e-001
At t = 4.0000e+006      y = 5.162636e-004   2.066108e-009   9.994837e-001
At t = 2.0793e+007      y = 1.000000e-004   4.000397e-010   9.999000e-001
    rootsfound[] =  -1   0
At t = 4.0000e+007      y = 5.201163e-005   2.080572e-010   9.999480e-001
At t = 4.0000e+008      y = 5.189595e-006   2.075849e-011   9.999948e-001
At t = 4.0000e+009      y = 5.264734e-007   2.105895e-012   9.999995e-001
At t = 4.0000e+010      y = 5.252772e-008   2.101109e-013   9.999999e-001

Final Statistics:
nst = 566    nfe  = 812    nsetups = 108    nfeLS = 0      nje = 11
nni = 808    ncfn = 0      netf = 25     nge = 594

@acroy
Copy link
Contributor

acroy commented Jun 1, 2014

The difference to Julia is probably due to the different choice of tolerances. In Sundials.ode we use scalar reltol=1e-4 and abstol=1e-6, while the Sundials example uses reltol=1e-4 and a vector [1e-8, 1e-14, 1e-6] for the absolute tolerances...

acroy added a commit that referenced this pull request Jun 1, 2014
@acroy acroy merged commit 00d0f6d into SciML:master Jun 1, 2014
@tkelman tkelman deleted the bindeps branch June 1, 2014 14:08
@tkelman
Copy link
Contributor Author

tkelman commented Jun 1, 2014

Makes sense about the tolerances. The non-simplified examples look to have tolerances that match the C versions, but also not exactly the same results.

Anyway, great to get more packages easy to install. Next is making a test/runtests.jl file that at a bare minimum runs the examples to check that they don't error or crash, and hopefully checks that the results are right too. Then we can turn on Travis for Linux (and/or Mac if we feel like it), AppVeyor for Windows, or just let PackageEvaluator flag any problems.

@acroy
Copy link
Contributor

acroy commented Jun 1, 2014

Within the tolerances the results should agree. I modified Sundials.ode to take a tolerance vector and on my Mac this seems to be the case (compared to cvRoberts_dns.out).

I agree that we should add some tests. I am not entirely sure how to start. We could have some "simple" ODEs for which we know the solution (like in ODE.jl) or we could use the simplified examples as you say.

acroy added a commit that referenced this pull request Jun 12, 2014
Added installation "instructions" reflecting the BinDeps integration in #12.
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.

None yet

4 participants