Added a build recipe for Simbody 3.5.3. #1076

moorepants · 2016-07-17T19:08:55Z

This is a minimal Simbody installation for all three platforms.

TODO

Figure out the activate error on Windows.
Deal with the Mac rpath warning (either apply a patch from Simbody dev or ...)

This is a minimal Simbody installation for all three platforms.

conda-forge-linter · 2016-07-17T19:08:58Z

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/libgfortran, recipes/pytrie, recipes/simbody, recipes/subprocess32) and found some lint.

Here's what I've got...

For recipes/simbody:

The recipe must have some tests.

moorepants · 2016-07-17T19:10:06Z

@chrisdembia Have a look at this. I'm going to try to get everything compiled for conda with Simbody and Opensim.

conda-forge-linter · 2016-07-17T19:10:50Z

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/simbody) and found some lint.

Here's what I've got...

For recipes/simbody:

The recipe must have some tests.

ocefpaf · 2016-07-17T19:13:35Z

recipes/simbody/meta.yaml

+        - blas    # [not win]
+        - lapack  # [not win]
+        - vc 14   # [win]
+


@moorepants you can add a simple "file exists" test here. See an example in:

https://github.com/conda-forge/hdfeos2-feedstock/blob/master/recipe/meta.yaml#L26-L27

conda-forge-linter · 2016-07-17T19:18:37Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/simbody) and found it was in an excellent condition.

moorepants · 2016-07-17T19:27:25Z

@sherm1 conda-forge does not yet have libxi and libxmu built as binaries, so this doesn't include those optional dependencies. I'd like to make conda packages here for libxi and libxmu so that the visualizer works. Do these two libraries build easily on Windows? How do you typically distrubute the visualizer dependencies for Simbody on Windows?

Also, how can I ensure that Simbody links against a lapack and blas that we have installed. Is that possible or do you ship those with Simbody and we have to use your version?

chrisdembia · 2016-07-18T06:47:42Z

Thank you for doing this!

@klshrinidhi may be interested.

How do you typically distrubute the visualizer dependencies for Simbody on Windows?

It seems that there are no visualizer dependencies on Windows.

Also, how can I ensure that Simbody links against a lapack and blas that we have installed. Is that possible or do you ship those with Simbody and we have to use your version?

Simbody does come with blas/lapack on Windows but it is fine to use other blas/lapack. This is controlled by setting the cmake variable BUILD_USING_OTHER_LAPACK. I think it should be set to a semicolon-separated list of the libraries to use (either the names or the full paths?).

@moorepants do you plan on packaging released versions of Simbody/OpenSim? I'm not sure it is a good idea to package the development version of OpenSim; it is changing a lot and should not be used for research in its current state. IMO it's fine to package the current development version of Simbody (i.e., master branch). EDIT: I can see now that you only plan on packaging released versions.

chrisdembia · 2016-07-18T06:48:05Z

recipes/simbody/meta.yaml

+
+# NOTE: This installs the minimum dependencies for Simbody, i.e., no
+# visualizer. I'm also not completely sure if blas and lapack are necessary for
+# the unix builds because they are supposedly included with Simbody. Once


blas/lapack are not included on UNIX; they are only included on Windows.

Ok, I'll remove this note.

moorepants · 2016-07-18T15:56:15Z

@isuruf The build is failing on non py35 instances on appveyor. What do you do to handle that?

isuruf · 2016-07-18T15:58:22Z

recipes/simbody/meta.yaml

+
+build:
+    number: 0
+    skip: true    # [win and py35]


This should be win and not py35

moorepants · 2016-07-18T19:01:43Z

@chrisdembia

Thank you for doing this!

NP, the combination of a matured conda-forge build system and inspiration at SciPy 2016 finally got me to do it.

It seems that there are no visualizer dependencies on Windows.

Ok. That makes it easy.

Simbody does come with blas/lapack on Windows but it is fine to use other blas/lapack. This is controlled by setting the cmake variable BUILD_USING_OTHER_LAPACK. I think it should be set to a semicolon-separated list of the libraries to use (either the names or the full paths?).

I'll use Simbody's lapack for Windows and the conda provided ones for Mac and Linux.

@moorepants do you plan on packaging released versions of Simbody/OpenSim? I'm not sure it is a good idea to package the development version of OpenSim; it is changing a lot and should not be used for research in its current state. IMO it's fine to package the current development version of Simbody (i.e., master branch). EDIT: I can see now that you only plan on packaging released versions.

Correct, for now I'll only package released versions, but conda-forge supports having dev builds on pre-release channels. We could set that up too later.

jakirkham · 2016-07-18T19:16:17Z

recipes/simbody/bld.bat

@@ -0,0 +1,8 @@
+mkdir build
+cd build
+cmake -G "%CMAKE_GENERATOR%" -DBUILD_VISUALIZER=OFF -DCMAKE_INSTALL_PREFIX="%LIBRARY_PREFIX%"


I think %CMAKE_GENERATOR% can't be quoted.

jakirkham · 2016-07-19T22:56:51Z

I was about to recommend that we set detect_binary_files_with_prefix to true in the build section.

Interesting. I'm not sure that feature does exactly what we want, since it's the install directory that gets baked in, not the build directory. Though these terms might mean different things to Conda vs CMake.

Ah, but that's not the case. Everything gets installed into a build prefix. Only after that has occurred is this detected and fixed. Then it is packaged into a tarball for distribution. So I do think this will give us what we want.

moorepants · 2016-07-19T22:58:21Z

I'm trying the flag locally, will push if it works.

chrisdembia · 2016-07-19T22:58:46Z

Everything gets installed into a build prefix

Ah excellent; thank you for explaining.

moorepants · 2016-07-19T23:27:39Z

Ok, the flag in the meta.yaml files works. I can now run the examples for simbody within a conda env.

One more thing though. I had to manually set LD_LIBRARY_PATH to /home/moorepants/miniconda/envs/simbody-test/lib for things to properly link. @jakirkham Is there something in conda that sets this for you or rewrites paths so that it isn't needed? It'd be nice if users didn't have to do this.

Also, when in a conda env, will the compiler look into /home/moorepants/miniconda/envs/simbody-test/include for headers automatically? i.e. does conda help redirect things there?

jakirkham · 2016-07-19T23:32:20Z

So, it sounds like you were trying to build something else after building Simbody, correct? Possibly something that was linking to it? Could you please explain how you were doing this build?

moorepants · 2016-07-19T23:39:31Z

Yes, simbody is a library and you typically write a C++ file with your simulation model and then link to the simbody lib when compiling. Here is what I did:

$ conda create -n simbody-test simbody
$ source activate simbody-test
(simbody-test)$ cp simbody-example-from-readme example.cpp
(simbody-test)$ # create simple CMakeLists.txt like https://github.com/simbody/simbody/blob/master/cmake/SampleCMakeLists.txt
(simbody-test)$ export LD_LIBRARY_PATH=~/miniconda/envs/simbody-test/lib
(simbody-test)$ cmake .
(simbody-test)$ make
(simbody-test)$ ./example

This will open up a simple GUI that shows the example animation of the system. I'm curious if there is a way to avoid have to set LD_LIBRARY_PATH. If I installed simbody system wide it would install to /usr/local by default and I don't think I'd have to set LD_LIBRARY_PATH because the lib is in a standard location. Does conda magically make it's environment the "standard location"?

jakirkham · 2016-07-19T23:46:46Z

I think you need to specifying some more info to CMake. For instance, where the libraries live and similar.

moorepants · 2016-07-20T00:29:54Z

Should I disable the windows builds and we merge this? If so, what's the best way to do that?

jakirkham · 2016-07-20T00:35:45Z

recipes/simbody/meta.yaml

+
+build:
+    number: 200
+    skip: true                # [win and not py35]


Change the selector win and not py35 -> win.

Probably a [skip appveyor] would be nice too.

...the last part refers to the commit message if that was not obvious. Sorry it was a bit vague.

jakirkham · 2016-07-20T00:37:30Z

SGTM. The changes to skip Windows are noted above.

Great work on this all. Will be a very nice addition to conda-forge.

jakirkham · 2016-07-20T01:15:45Z

recipes/simbody/meta.yaml

+    - python 3.5.*  # [win and py35]
+  commands:
+    - if not exist %LIBRARY_INC%\\simbody\\Simbody.h exit 1  # [win]
+    - test -f $PREFIX/include/simbody/Simbody.h              # [unix]


We should probably have more file exists tests. Some tests for the libraries and binaries would be good.

jakirkham · 2016-07-21T07:33:22Z

It seems to be using lib64 on Linux. We need to fix it to use lib or move the contents over.

chrisdembia · 2016-07-21T17:27:09Z

I think you can use CMAKE_INSTALL_LIBDIR to control that.

chrisdembia · 2016-07-21T17:27:39Z

We use CMake's GNUInstallDirs to determine the correct platform-dependent install directories.

jakirkham · 2016-07-21T17:29:23Z

We use CMake's GNUInstallDirs to determine the correct platform-dependent install directories.

So will overriding with CMAKE_INSTALL_LIBDIR still be respected then?

chrisdembia · 2016-07-21T17:36:52Z

Depends on if those variables are cmake cache variables or not. I think changes made by the user will be respected but I am not certain.

moorepants · 2016-07-21T19:34:09Z

That cmake flag didn't seem to force the files to be in lib instead of lib64.

moorepants · 2016-07-21T19:35:04Z

Oh sorry, i must have been looking at an old ci run. It looks like it worked.

jakirkham · 2016-07-21T20:37:55Z

LGTM

isuruf · 2016-07-22T01:38:45Z

recipes/simbody/meta.yaml

+    - if not exist %LIBRARY_INC%\\simbody\\SimTKlapack.h exit 1       # [win]
+    - if not exist %LIBRARY_INC%\\simbody\\SimTKmath.h exit 1         # [win]
+    - if not exist %LIBRARY_INC%\\simbody\\SimTKsimbody_aux.h exit 1  # [win]
+    - if not exist %LIBRARY_INC%\\simbody\\SimTKsimbody.h exit 1      # [win]


Windows failure was because of this difference, https://github.com/simbody/simbody/blob/master/CMakeLists.txt#L567-L579

Headers and visualizer are installed in different directories than Unixes, but IMO, they shouldn't.

but IMO, they shouldn't.

You mean for conda, specifically? I think it makes sense for conda to use the same installation layout for all platforms, but in general, different platforms have different requirements and conventions.

I'm fine with having Simbody's SIMBODY_VISUALIZER_REL_INSTALL_DIR and SIMBODY_INCLUDE_INSTALL_DIR being cache variables so that packagers can customize their values.

Makes sense. Some of the reasons mentioned there doesn't apply to conda, but making those two cache variables should fix it.

On second thought, it should not be different in general. For eg: a user would do,

#include <simbody/Simbody.h>

in user program. This would work in Unixes, but will not in Windows because Simbody.h is installed in C:\Program Files\simbody\include\Simbody.h

A user should not use #include <simbody/Simbody.h>, and in general this won't work unless they install Simbody into /usr. As shown in Simbody examples, the user should use #include "Simbody.h" (or #include <Simbody.h>). This will work cross-platform because of the way we create a cmake Config file / pkg-config file. Both of these files are configured by CMake to provide the appropriate include dir for the platform you're on.

We expect that people building a project that uses Simbody will use CMake to set up the appropriate compiler/linker flags, etc. If a downstream project to Simbody chooses not to use CMake (or pkg-config) to set up the flags, then yes, the project will have to specify their own platform-dependent behavior.

On UNIX, it is conventional to install all headers inside a <project-name> folder (e.g., $prefix/include/simbody), since the project is often installed in a system-wide directory. This is not the case on Windows (aside from conda), so an extra simbody directory would be weird.

You are right. Project config file does include $PREFIX/include/simbody in it, so it should be good. pkg-config is not though, but it's unrelated to this PR.

We'll merge this as is fr Windows and once the conda activate bug is addressed we can get the Windows build working.

jakirkham · 2016-07-22T06:30:33Z

Thanks @moorepants for putting this together. Also thanks @chrisdembia, @isuruf, and @kmuehlbauer for your helpful reviews. Now let's get this into a feedstock. 😄

chrisdembia · 2016-07-22T06:33:48Z

🎆 Thanks everyone.

Added a build recipe for Simbody 3.5.3.

e29d9c4

This is a minimal Simbody installation for all three platforms.

Merge branch 'master' into simbody

6271d12

ocefpaf reviewed Jul 17, 2016
View reviewed changes

Added a test to check for Simbody.h.

f850086

Added unix toolchain, default conda env, and build dir.

5ff19ed

Fixed Simbody.h check.

795ba77

chrisdembia reviewed Jul 18, 2016
View reviewed changes

isuruf reviewed Jul 18, 2016
View reviewed changes

moorepants closed this Jul 18, 2016

moorepants reopened this Jul 18, 2016

Fixed a number of things to reflect review comments.

c8acc2d

jakirkham reviewed Jul 18, 2016
View reviewed changes

Fix paths to the simbody-visualizer binary.

b1d6b7a

moorepants mentioned this pull request Jul 19, 2016

Hard-code relative path from SimTKsimbody library to simbody-visualizer simbody/simbody#512

Closed

jakirkham reviewed Jul 20, 2016
View reviewed changes

Disable windows builds for now [skip appveyor].

9ed61f2

jakirkham reviewed Jul 20, 2016
View reviewed changes

moorepants added 2 commits July 19, 2016 20:38

Added more file tests in the meta.yaml.

21e91ef

Fixed visualizer binary location.

9ee89cb

Force the linux lib dir to be lib instead of lib64.

fd28fd3

isuruf reviewed Jul 22, 2016
View reviewed changes

jakirkham merged commit 1777bd8 into conda-forge:master Jul 22, 2016

moorepants deleted the simbody branch April 5, 2018 16:08

Added a build recipe for Simbody 3.5.3. #1076

Added a build recipe for Simbody 3.5.3. #1076

Conversation

moorepants commented Jul 17, 2016 • edited Loading

conda-forge-linter commented Jul 17, 2016

moorepants commented Jul 17, 2016

conda-forge-linter commented Jul 17, 2016

Choose a reason for hiding this comment

conda-forge-linter commented Jul 17, 2016

moorepants commented Jul 17, 2016

chrisdembia commented Jul 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moorepants commented Jul 18, 2016

Choose a reason for hiding this comment

moorepants commented Jul 18, 2016 • edited Loading

Choose a reason for hiding this comment

jakirkham commented Jul 19, 2016 • edited Loading

moorepants commented Jul 19, 2016

chrisdembia commented Jul 19, 2016

moorepants commented Jul 19, 2016

jakirkham commented Jul 19, 2016

moorepants commented Jul 19, 2016

jakirkham commented Jul 19, 2016

moorepants commented Jul 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Jul 20, 2016

Choose a reason for hiding this comment

jakirkham commented Jul 21, 2016

chrisdembia commented Jul 21, 2016

chrisdembia commented Jul 21, 2016

jakirkham commented Jul 21, 2016

chrisdembia commented Jul 21, 2016

moorepants commented Jul 21, 2016

moorepants commented Jul 21, 2016

jakirkham commented Jul 21, 2016

Choose a reason for hiding this comment

chrisdembia Jul 22, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisdembia Jul 22, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Jul 22, 2016

chrisdembia commented Jul 22, 2016

moorepants commented Jul 17, 2016 •

edited

Loading

chrisdembia commented Jul 18, 2016 •

edited

Loading

moorepants commented Jul 18, 2016 •

edited

Loading

jakirkham commented Jul 19, 2016 •

edited

Loading

chrisdembia Jul 22, 2016 •

edited

Loading

chrisdembia Jul 22, 2016 •

edited

Loading