search

erwincoumans / tiny-differentiable-simulator

Tiny Differentiable Simulator is a header-only C++ and CUDA physics library for reinforcement learning and robotics with zero dependencies.
Apache License 2.0
1.2k stars 129 forks source link

Feature/eigen/cpp_ad for py #78

Closed erwincoumans closed 3 years ago

google-cla[bot] commented 3 years ago

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

ErikGartner commented 3 years ago

@googlebot I consent.

ErikGartner commented 3 years ago

I synced the branch with master.

erwincoumans commented 3 years ago

Thanks, triggering CI again (PAGMO2 should be off by default, otherwise CI fails)

erwincoumans commented 3 years ago

Thanks @ErikGartner , I added some comments in-line, could you have a look?

ErikGartner commented 3 years ago

Hi @erwincoumans! Sorry I missed the notification from Github. I'll have a look and get back to you asap. /Erik

ErikGartner commented 3 years ago

Hi @erwincoumans, I have fixed your comments now. Let me know if there is anything else. :-)

erwincoumans commented 3 years ago

In the python/CMakeListst.txt, we need to add IF(USE_CPPAD) before set_target_properties on a project.

ErikGartner commented 3 years ago

@erwincoumans Should be fixed now in 88b66dc.

erwincoumans commented 3 years ago

Thanks for the contribution, I'll merge and check it out. Did you also have PD control for spherical joints?

erwincoumans commented 3 years ago

When running python setup.py build on Windows, it fails.

ffsim_ad.cc /Fobuild\temp.win-amd64-3.7\Release\python/pytinydiffsim_ad.obj -fpermissive -DPYBULLET_USE_NUMPY -DWIN32 -DGLEW_STATIC /std:c++17
cl : Command line warning D9002 : ignoring unknown option '-fpermissive'
pytinydiffsim_ad.cc
python/pytinydiffsim_ad.cc(17): fatal error C1083: Cannot open include file: 'cppad/cg/cppadcg.hpp': No such file or directory

After adding the third_party/cppadcodegen/include folder, it fails to find some files that are autogenerated during a complex build, so we likely cannot easily get compilation inside setup.py working (without relying on any other build system)

cannot open source file "cppad/cg/model/threadpool/openmp_c.hpp"

Probably best to disable pytinydiffsim_ad.cc for now, unless we can easily build cppad etc from within setup.py (note that it doesn't use cmake or precompiled libs)

UPDATE: It looks like providing the precompiled headers for Windows may work, I will check it out.

ErikGartner commented 3 years ago

I think we need to update the Python examples since I changed the API to only expose one type of data types., i.e. Quaternion that either compile to TinyQuaternion or Eigen::Quaternion depending on the interface.

I tried to have both both but pybind11 didn't like having TinyQuaternion and at the same type compiling Quaternion to TinyQuaternion.

erwincoumans commented 3 years ago

Good point, haven't tried running the examples recently. It would be good to fix them, and also add a Python example for the new CppAd (or is there already one)?