opensim-org / opensim-core

SimTK OpenSim C++ libraries and command-line applications, and Java/Python wrapping.
https://opensim.stanford.edu
Apache License 2.0
764 stars 309 forks source link

Enable advanced users to modify binding/wrapping for access to non-exposed classes/methods #3414

Closed aymanhab closed 1 year ago

aymanhab commented 1 year ago

This would be a backdoor for swig failure/overlooked classes/methods and to enable users to access simbody methods not available through the swig interface as described in this forum thread https://simtk.org/plugins/phpBB/viewtopicPhpbb.php?f=91&t=15254&p=0&start=0&view=&sid=0cf16905bd2b7d10252faa4b0cb17a67

jsn54 commented 1 year ago

To address the issue described in the post mentioned above, it might be possible to create, via SWIG, a tiny Python which would only rely on C++ code to execute the expected function of the hidden class and retrieve its result into an OpenSim compatible format. The attached files could be the basis of a first attempt :

aymanhab commented 1 year ago

Thanks for getting started on this @jsn54 You actually don't need to have a separate cpp and h file and all the complications that come with that. I started a quick and dirty PR here https://github.com/opensim-org/opensim-core/pull/3416 that does what you want as an extension to the Model class in the .i file.

With a little more work you can promote this method to a new utility class (e.g. GravityWrapper) rather than injecting code into the OpenSim::Model class. At any rate I'd try as much as possible to leverage the .i files and avoid creating a separate class with header and cpp files. Please let me know how that works for you.

aymanhab commented 1 year ago

I'd add that you shouldn't create a new Gravity force class/instance, you should retrieve that from the Model instead.

jsn54 commented 1 year ago

Thank you very much for your answers and hints.

Let me please sum up the info you gave, just to check that I've correctly understood the ins / outs, pro / cons and further steps:

Thanks in advance for your help !

De: "Ayman Habib" @.> À: "opensim-org/opensim-core" @.> Cc: "jonathan" @.>, "Mention" @.> Envoyé: Mercredi 1 Mars 2023 23:03:12 Objet: Re: [opensim-org/opensim-core] Enable advanced users to modify binding/wrapping for access to non-exposed classes/methods (Issue #3414)

I'd add that you shouldn't create a new Gravity force class/instance, you should retrieve that from the Model instead.

— Reply to this email directly, [ https://github.com/opensim-org/opensim-core/issues/3414#issuecomment-1450910783 | view it on GitHub ] , or [ https://github.com/notifications/unsubscribe-auth/ACQ6TJFJITQGPX2U6CESIS3WZ7BSBANCNFSM6AAAAAAVJW6CGA | unsubscribe ] . You are receiving this because you were mentioned. Message ID: @.***>

==============================================================================

JSN, 20230227

This is the root-directory CMake file for building a SWIG-Python wrapper to

enable OpenSim-based Python scripts to access simBody hidden classes and

functions.

==============================================================================

--- CMakeLists.txt ---

This is the minimum version of CMake that OpenSim requires:

cmake_minimum_required(VERSION 3.2) project(LightGravityWrapper)

OpenSim requires a compiler that supports C++11.

set(CMAKE_CXX_STANDARD 11) set(CMAKE_CXX_STANDARD_REQUIRED ON)

OpenSim stuff

set(OPENSIM_INSTALL_DIR "C:/OpenSim 4.4/cmake") set(OPENSIM_INC_DIRS "C:/OpenSim 4.4/sdk/include/OpenSim/" "C:/OpenSim 4.4/sdk/include/OpenSim/Common")

find_package(OpenSim 4.4 REQUIRED HINTS "${OPENSIM_INSTALL_DIR}")

===========================================================

First target: a library (to check that the code compiles)

===========================================================

add_library(LibGravityWrapper LightGravityWrapper.cpp) target_link_libraries(LibGravityWrapper osimCommon)

================================

Second target: the swig module

================================

find_package(Python COMPONENTS Development Interpreter)

if (Python_FOUND) message("Found Python: ${Python_SITELIB}") # print sitepath else() message("No Python found.") return() endif()

find_package(SWIG) include(UseSWIG)

if(SWIG_FOUND) message("Found SWIG ${SWIG_VERSION}") else() message("No SWIG found.") return() endif()

set(UseSWIG_TARGET_NAME_PREFERENCE STANDARD)

set_property(SOURCE LightGravityWrapper.i PROPERTY CPLUSPLUS ON)

message("Binary dir: ${CMAKE_SOURCE_DIR}/build") set(BUILD_DIR ${CMAKE_CURRENT_BINARY_DIR}/SwigGravityWrapper)

swig_add_library(swigGravityWrapper TYPE SHARED LANGUAGE python OUTPUT_DIR ${BUILD_DIR} # important! else find_packages() wont find SOURCES LightGravityWrapper.i ) target_include_directories(swigGravityWrapper PUBLIC ${OPENSIM_INC_DIRS}) target_link_libraries(swigGravityWrapper osimCommon)

set_property(TARGET swigGravityWrapper PROPERTY LIBRARY_OUTPUT_DIRECTORY ${BUILD_DIR})

jsn54 commented 1 year ago

Hi everyone, after many steps of "trial and error" caused by my lack in CMake-swig, please attached find a minimum version of a swig-based GravityWrapper. An executable is build, it works fine. SWIG process seems to work as well (compile/link/.so library build), however I can't import the module in a python interpreter. Would anyone examine this draft version and correct / tell me how to correct it ? Thanks in advance ! MinimumGravityWrapper.zip

aymanhab commented 1 year ago

@jsn54 As I pointed out, sorry if that wasn't clear, you don't need a separate cpp file which require a big fight with cmake to get right and integrate. Instead keep all the implementation in a header file and include it in the .i file thus reusing the existing build system as is. For any new development like this I'd use the main branch and latest tools (vstudio, etc.) not the 4.4 codebase unless you're willing to fight with these as it's likely some new code/features may not compile with earlier tools.

aymanhab commented 1 year ago

@jsn54 Following up to see if you were able to get past your swig/compilation problems so we can close this issue. Thank you

jsn-inrs commented 1 year ago

I'm sorry this topic was closed yesterday. Unfortunately, I haven't been able to go through swig/compilation issues, as I wrote in PR3416 which is closely linked to this topic. Actually, I was lost between those topics on github et their counterparts on OpenSim forums. Would someone please help me have a correct cmake file in order to compile a little swig-python module ? Either through this approach (swig-python module) or the one described in #PR3416 (recompile opensim python bindings), I was not able to carry out how to retrieve gravity forces in python. And in both cases only because compilation in Windows is a ugly mess !

aymanhab commented 1 year ago

Hello, I just updated the PR with my earlier suggestion (https://github.com/opensim-org/opensim-core/pull/3416) with no necessary cmake changes. The signature of the methods in GravityHelper.java seem correct please give it a try when the PR is done building or let me know if you run into problems using it.

aymanhab commented 1 year ago

@jsn-inrs I haven't heard back from you so I'll assume you were able to proceed and extract what you want from simbody, if not please comment here and reopen as needed. Thank you

jsn-inrs commented 1 year ago

Thank you for regularly following the progress of this PR. My schedule has been very busy this month and I was only able to start testing the compilation of the updated version of the OpenSim code yesterday. I'll keep you posted on my progress over the next few days. Thanks again for your attention to this request.

jsn-inrs commented 1 year ago

It's already friday evening, unfortunately I didn't manage to have my script retrieve gravity forces. Would it be possible to carry on discussing the practical actions by means of mail or (even better) video conference (and screen sharing) ? I know opensim fellows are over-busy, but it may be more efficient and time saving to save a date in our agenda. Thanks in advance.

jsn-inrs commented 1 year ago

Hi everyone, I've tried the PR3414 and 3416 but what I plan is more complex. I think that SWIG wrapping is the best way to access and process Gravity Forces features. I've created a new project available on github : https://github.com/jsn-inrs/GravForcesProcessor Would you please help me building a working Python package ? At least for what is linked to SWIG configuration of the Makefile (spdlog and OpenSim dependencies...). I lack skills but I'm really eager to learn and contribute then to OpenSim's development. Thanks in advance !