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

add gtsam #14339

Closed
wants to merge 6 commits into from
Closed

add gtsam #14339

wants to merge 6 commits into from

Conversation

wolfv
Copy link
Member

@wolfv wolfv commented Mar 22, 2021

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge-linter
Copy link

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/gtsam) and found some lint.

Here's what I've got...

For recipes/gtsam:

  • The recipe must have some tests.
  • There are too few lines. There should be one empty line at the end of the file.

For recipes/gtsam:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

@ameysutavani ameysutavani mentioned this pull request Mar 24, 2021
9 tasks
@venabled
Copy link

Pulling your recipe @wolfv , in the tests looks like ceres-solver 2.0 release broke something:

[199/501] Building CXX object gtsam/nonlinear/tests/CMakeFiles/testAdaptAutoDiff.dir/testAdaptAutoDiff.cpp.o
FAILED: gtsam/nonlinear/tests/CMakeFiles/testAdaptAutoDiff.dir/testAdaptAutoDiff.cpp.o 
$BUILD_PREFIX/bin/x86_64-conda-linux-gnu-c++ -DBOOST_ALL_NO_LIB -DBOOST_CHRONO_DYN_LINK -DBOOST_DATE_TIME_DYN_LINK -DBOOST_FILESYSTEM_DYN_LINK -DBOOST_REGEX_DYN_LINK -DBOOST_SERIALIZATION_DYN_LINK -DBOOST_SYSTEM_DYN_LINK -DBOOST_THREAD_DYN_LINK -DBOOST_TIMER_DYN_LINK -DNDEBUG -DTOPSRCDIR=\"$SRC_DIR\" -I../gtsam/3rdparty/metis/include -I../gtsam/3rdparty/metis/libmetis -I../gtsam/3rdparty/metis/GKlib -I../gtsam/3rdparty/SuiteSparse_config -I../gtsam/3rdparty/CCOLAMD/Include -I../ -I. -I../CppUnitLite -I$PREFIX/include/eigen3 -fvisibility-inlines-hidden -std=c++17 -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/gtsam-4.0.3 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix -Wno-misleading-indentation -O3 -DNDEBUG -Wall -fPIC -O3 -Wno-unused-local-typedefs -MD -MT gtsam/nonlinear/tests/CMakeFiles/testAdaptAutoDiff.dir/testAdaptAutoDiff.cpp.o -MF gtsam/nonlinear/tests/CMakeFiles/testAdaptAutoDiff.dir/testAdaptAutoDiff.cpp.o.d -o gtsam/nonlinear/tests/CMakeFiles/testAdaptAutoDiff.dir/testAdaptAutoDiff.cpp.o -c ../gtsam/nonlinear/tests/testAdaptAutoDiff.cpp
In file included from ../gtsam/nonlinear/tests/testAdaptAutoDiff.cpp:21:
../gtsam/nonlinear/AdaptAutoDiff.h:23:10: fatal error: ceres/autodiff.h: No such file or directory
   23 | #include <ceres/autodiff.h>

@ameysutavani
Copy link
Contributor

ameysutavani commented Apr 26, 2021

@wolfv I tried making a few test changes in #14363 but the recipe still seems to fail. I am quite new to conda-forge and cannot seem to now find the build-log to investigate further. Can you please help by taking a second look at either this PR or #14363

Please do let me know how I can help 😄

@wolfv
Copy link
Member Author

wolfv commented May 4, 2021

Hi @ameysutavani I tried again, building against boost 1.76 (which will be the next conda-forge supported version). This won't be compatible with the RoboStack packages, but it's a start to get gtsam in and maybe at some point we'll rebuild the RoboStack to also use boost 1.76

@ameysutavani
Copy link
Contributor

ameysutavani commented Jul 10, 2021

@wolfv Touch-basing on this thread again after a while; I tried solving the ceres autodiff.h issues that @venabled had pointed out here: #15578

The TL;DR status is:

  1. GTSAM is using a very old version of ceres-solver in its 3rdparty folder. The newer ceres versions have broken compatibility in the file location (autodiff.h is moved to ceres/internal/autodiff.h) as well as the structs used (ceres::internal::AutoDiff no longer exists). This makes it very hard to un-vendor ceres-solver as a 3rdparty dependency from GTSAM. Ideally, this issue should be fixed upstream in GTSAM. It also is not just limited to ceres-solver as explained here: [Feature Request] Allow use of system libraries for 3rdparty codebases borglab/gtsam#292

  2. Given the above situation, I can think of following options that we can try:

A. Allow vendoring of GTSAM's 3rdparty ceres: The GTSAM user will need to be mindful that this may create conflicts with
the system/conda-forge installed versions of dependencies. I did try building this recipe; it seems to pass building and testing but is failing in the packaging step here. Any help here understanding what might be going wrong is much appreciated 😄

B. Wait for these dependencies to fixed upstream: This is probably not going to happen anytime soon.

Would like to hear if there can be any more options that can be considered here from both @wolfv and @venabled ;
Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants