Skip to content

Commit

Permalink
[core] Improve code quality.
Browse files Browse the repository at this point in the history
* [core] Allow arbitrary larger controller update period.
* [core] Fix deprecation and unused variable warnings.
* [core] Improve error reporting by printing fully qualified function name without arguments.
* [core] Remove irrelevant virtual keywords.
* [core] Use randomPermutationPeriod=0 to disable random permutation of 'PGSSolver' iterative solver, instead of 'isfinite', which is ill-defined for integers.
* [core] Do NOT shuffle first iteration cycle of 'PGSolver', and initialize indices in reverse order to make convergence (hopefully) faster.
* [core] Enable implicit conversion warnings.
* [core] More consistent coding style.
* [core] Avoid static_cast if possible and use safer conversion if possible.
* [core] Remove all C-style cast and enable warning to disallow them.
* [core] Replace (u)int32_t by std::size_t/ptrdiff_t when appropriate for clarity and portability.
* [core] Fix visco-elastic coupling forces.
* [core] Remove all dependencies to jsoncpp in core headers.
* [core/python] Add fallback to completary unsigned/signed type for integers for 'convertFromPython'.
* [gym/common/envs] Refresh action space before observation space since action could be part of observation but not conversely.
* [gym/common/wrappers] Relax argument type of 'FilteredFrameStack' for convenience.
* [gym/common/wrappers] Rename 'FilterFrameStack' in 'FilteredFrameStack' for clarity.
* [gym/examples] Automatically monitor env step 'info' on Tensorboard's histograms when using RLlib backend.
* [gym/examples] Only log data in Tensorboard format when RLlib is used, to dramatically reduce log folder size.
* [misc] Minor update of easy install dependencies.
* [misc] RLlib 'evaluate' helper method now returns agregated info over episode.
* [misc] Relax Ubuntu version check for easy install script since it should work on any release in practice.
* [misc] Do NOT install robotpkg-urdfdom binaries since 'liburdom-dev' is already installed by other easy-install dependencies.
* [misc] More robust Python library finding in provided CMake configuration file.
* [misc] Check compatibility with required jiminy version if any in provided CMake configuration file.
* [misc] Detect Numpy headers in provided CMake configuration file.
* [misc] Add missing boost definitions in CMake configuration file.
  • Loading branch information
duburcqa authored Apr 24, 2021
1 parent 499f842 commit 8cc93c8
Show file tree
Hide file tree
Showing 88 changed files with 1,096 additions and 984 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ubuntu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ jobs:
#####################################################################################

- name: Python linter and static type checker
# Ubuntu18 is distributed with Python3.6, which is not supported by Numpy>=1.20.
# The new type check support of Numpy is raising pylint and mypy errors, so Ubuntu18
# Ubuntu 18 is distributed with Python3.6, which is not supported by Numpy>=1.20.
# The new type check support of Numpy is raising pylint and mypy errors, so Ubuntu 18
# is used to do type checking for now.
if: matrix.os == 'ubuntu-18.04'
run: |
Expand Down
5 changes: 2 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
cmake_minimum_required(VERSION 3.10)

# Set the build version
set(BUILD_VERSION 1.6.11)
set(BUILD_VERSION 1.6.12)

# Extract major, minor and patch version
string(REPLACE "." ";" _VERSION "${BUILD_VERSION}")
Expand Down Expand Up @@ -31,8 +31,7 @@ include(${CMAKE_SOURCE_DIR}/build_tools/cmake/buildPythonWheel.cmake)
# Set the compilation flags
if(NOT WIN32)
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS} -O0 -g -ftemplate-backtrace-limit=0 ${WARN_FULL}")
set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS} -DNDEBUG -O3 -Wfatal-errors -Werror ${WARN_FULL} \
-Wno-non-virtual-dtor -Wno-deprecated-declarations -Wno-unused-parameter")
set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS} -DNDEBUG -O3 -Wfatal-errors -Werror ${WARN_FULL}")
else()
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS} /EHsc /bigobj -g /Wall")
# It would be great to have the same quality standard for Windows but
Expand Down
8 changes: 4 additions & 4 deletions INSTALL.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Linux-based OS

## Easy-install procedure on Ubuntu 14/16/18/19, and Debian 8/9
## Easy-install procedure on Ubuntu 18/19/20, and Debian 8/9

### Jiminy

Expand Down Expand Up @@ -70,12 +70,12 @@ Installing the Python packages `ray==1.0.0` are required to run some of the prov
python -m pip install gym-jiminy
```

## Build Jiminy from source on Ubuntu 18/20 (excluding dependencies)
## Build Jiminy from source on Ubuntu 18+ (excluding dependencies)

First, one must install the pre-compiled libraries of the dependencies. Most of them are available on `robotpkg` APT repository. Just run the bash script to install them automatically for Ubuntu 18/20. It should be straightforward to adapt it to any other distribution for which `robotpkg` is available.
First, one must install the pre-compiled libraries of the dependencies. Most of them are available on `robotpkg` APT repository. Just run the bash script to install them automatically for Ubuntu 18 and upward. It should be straightforward to adapt it to any other distribution for which `robotpkg` is available.

```bash
sudo ./build_tools/easy_install_deps_ubuntu18.sh
sudo ./build_tools/easy_install_deps_ubuntu.sh
```

You are now ready to build and install Jiminy itself.
Expand Down
4 changes: 2 additions & 2 deletions build_tools/build_install_deps_linux.sh
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,10 @@ git apply --reject --whitespace=fix "$RootDir/build_tools/patch_deps_linux/pinoc

# How to properly detect custom install of Boost library:
# - if Boost_NO_BOOST_CMAKE is TRUE:
# * Set the cmake cache variable BOOST_ROOT and Boost_INCLUDE_DIR
# - if Boost_NO_BOOST_CMAKE is FALSE:
# * Set the cmake cache variable CMAKE_PREFIX_PATH
# * Set the environment variable Boost_DIR
# - if Boost_NO_BOOST_CMAKE is FALSE:
# * Set the cmake cache variable BOOST_ROOT and Boost_INCLUDE_DIR

### Build and install the build tool b2 (build-ception !)
cd "$RootDir/boost"
Expand Down
4 changes: 2 additions & 2 deletions build_tools/build_install_deps_windows.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ git apply --reject --whitespace=fix "$RootDir/build_tools/patch_deps_windows/pin

# How to properly detect custom install of Boost library:
# - if Boost_NO_BOOST_CMAKE is TRUE:
# * Set the cmake cache variable BOOST_ROOT and Boost_INCLUDE_DIR
# - if Boost_NO_BOOST_CMAKE is FALSE:
# * Set the cmake cache variable CMAKE_PREFIX_PATH
# * Set the environment variable Boost_DIR
# - if Boost_NO_BOOST_CMAKE is FALSE:
# * Set the cmake cache variable BOOST_ROOT and Boost_INCLUDE_DIR

### Build and install the build tool b2 (build-ception !)
Set-Location -Path "$RootDir/boost"
Expand Down
34 changes: 0 additions & 34 deletions build_tools/cmake/FindNumPy.cmake

This file was deleted.

11 changes: 7 additions & 4 deletions build_tools/cmake/base.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,17 @@ set(WARN_FULL "-Wall -Wextra -Weffc++ -pedantic -pedantic-errors \
-Wcast-align -Wcast-qual -Wfloat-equal -Wformat=2 \
-Wformat-nonliteral -Wformat-security -Wformat-y2k \
-Wimport -Winit-self -Winvalid-pch -Wlong-long \
-Wmissing-field-initializers -Wmissing-format-attribute \
-Wmissing-noreturn -Wpacked -Wpointer-arith \
-Wredundant-decls -Wshadow -Wstack-protector \
-Wmissing-field-initializers -Wmissing-noreturn \
-Wmissing-format-attribute -Wctor-dtor-privacy \
-Wpointer-arith -Wold-style-cast -Wpacked \
-Woverloaded-virtual -Wredundant-decls \
-Wstrict-null-sentinel -Wshadow -Wstack-protector \
-Wstrict-aliasing=2 -Wswitch-default -Wswitch-enum \
-Wunreachable-code -Wunused -Wundef -Wlogical-op \
-Wdisabled-optimization -Wmissing-braces -Wtrigraphs \
-Wparentheses -Wwrite-strings -Werror=return-type \
-Wsequence-point -Wdeprecated") # -Wconversion
-Wsequence-point -Wdeprecated -Wconversion \
-Wdelete-non-virtual-dtor -Wno-non-virtual-dtor")

# Shared libraries need PIC
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
Expand Down
2 changes: 1 addition & 1 deletion build_tools/cmake/exportCmakeConfigFiles.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function(exportCmakeConfigFiles)

if(CMAKE_VERSION VERSION_GREATER "3.11.0")
set(COMPATIBILITY_VERSION SameMinorVersion)
else(CMAKE_VERSION VERSION_GREATER "3.11.0")
else()
set(COMPATIBILITY_VERSION AnyNewerVersion)
endif()

Expand Down
55 changes: 48 additions & 7 deletions build_tools/cmake/jiminyConfig.cmake
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
# Enable link rpath to find shared library dependencies at runtime
set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)

# Set Python find strategy to location by default
if(CMAKE_VERSION VERSION_GREATER "3.15")
cmake_policy(SET CMP0094 NEW)
endif()

# Get Python executable
if(DEFINED PYTHON_EXECUTABLE)
get_filename_component(_PYTHON_PATH "${PYTHON_EXECUTABLE}" DIRECTORY)
Expand All @@ -10,17 +15,27 @@ else()
if(CMAKE_VERSION VERSION_LESS "3.12.4")
find_program(Python_EXECUTABLE "python${PYTHON_REQUIRED_VERSION}")
else()
if(CMAKE_VERSION VERSION_LESS "3.14")
set(Python_COMPONENTS_FIND Interpreter)
else()
set(Python_COMPONENTS_FIND Interpreter NumPy)
endif()
unset(Python_EXECUTABLE)
unset(Python_EXECUTABLE CACHE)
unset(_Python_EXECUTABLE)
unset(_Python_EXECUTABLE CACHE)
if(PYTHON_REQUIRED_VERSION)
find_package(Python ${PYTHON_REQUIRED_VERSION} EXACT COMPONENTS Interpreter)
find_package(Python ${PYTHON_REQUIRED_VERSION} EXACT COMPONENTS ${Python_COMPONENTS_FIND})
else()
find_package(Python COMPONENTS Interpreter)
find_package(Python COMPONENTS ${Python_COMPONENTS_FIND})
endif()
endif()
endif()
if(NOT Python_EXECUTABLE)
if (jiminy_FIND_REQUIRED)
message(FATAL_ERROR "Python executable not found, CMake will exit.")
else()
set(jiminy_FOUND FALSE)
return()
endif()
endif()
Expand All @@ -29,23 +44,46 @@ endif()
execute_process(COMMAND "${Python_EXECUTABLE}" -c
"import importlib; print(int(importlib.util.find_spec('jiminy_py') is not None), end='')"
OUTPUT_VARIABLE jiminy_FOUND)
if (NOT jiminy_FOUND)
if(NOT jiminy_FOUND)
if (jiminy_FIND_REQUIRED)
message(FATAL_ERROR "`jiminy_py` Python module not found, CMake will exit.")
else()
return()
endif()
endif()

# Define Python_NumPy_INCLUDE_DIRS if necessary
if (NOT Python_NumPy_INCLUDE_DIRS)
execute_process(
COMMAND "${Python_EXECUTABLE}" -c
"import numpy; print(numpy.get_include(), end='')\n"
OUTPUT_VARIABLE __numpy_path)
find_path(Python_NumPy_INCLUDE_DIRS numpy/arrayobject.h
HINTS "${__numpy_path}" "${Python_INCLUDE_DIRS}" NO_DEFAULT_PATH)
endif()

# Get jiminy version, and check if compatible with requested version, if any.
# For now, compatibility is assumed as long as only patch version differs.
execute_process(COMMAND "${Python_EXECUTABLE}" -c
"import jiminy_py; print(jiminy_py.__version__, end='')"
OUTPUT_VARIABLE jiminy_VERSION)
string(REPLACE "." ";" _VERSION "${jiminy_VERSION}")
list(GET _VERSION 0 jiminy_VERSION_MAJOR)
list(GET _VERSION 1 jiminy_VERSION_MINOR)
list(GET _VERSION 2 jiminy_VERSION_PATCH)
if (PACKAGE_FIND_VERSION_MAJOR)
if (PACKAGE_FIND_VERSION_MAJOR EQUAL jiminy_VERSION_MAJOR)
message(FATAL_ERROR "Available `jiminy_py` version (${jiminy_VERSION}) not "
"compatible with requested one (${PACKAGE_FIND_VERSION}).")
endif()
endif()

# Find jiminy library and headers.
# Note that jiminy is compiled under C++17 using either old or new CXX11 ABI.
# Make sure very project dependencies are compiled for the same CXX11 ABI
# otherwise segfaults may occur. It should be fine for the standard library,
# but not for precompiled boost libraries such as boost::filesystem.
# https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html
execute_process(COMMAND "${Python_EXECUTABLE}" -c
"import jiminy_py; print(jiminy_py.__version__, end='')"
OUTPUT_VARIABLE jiminy_VERSION)
execute_process(COMMAND "${Python_EXECUTABLE}" -c
"import jiminy_py; print(jiminy_py.get_include(), end='')"
OUTPUT_VARIABLE jiminy_INCLUDE_DIRS)
Expand All @@ -54,7 +92,10 @@ execute_process(COMMAND "${Python_EXECUTABLE}" -c
OUTPUT_VARIABLE jiminy_LIBRARIES)

# Define compilation options and definitions
set(jiminy_DEFINITIONS EIGENPY_STATIC URDFDOM_STATIC HPP_FCL_STATIC PINOCCHIO_STATIC)
set(jiminy_DEFINITIONS
EIGENPY_STATIC URDFDOM_STATIC HPP_FCL_STATIC PINOCCHIO_STATIC
BOOST_MPL_CFG_NO_PREPROCESSED_HEADERS=ON BOOST_MPL_LIMIT_VECTOR_SIZE=30 BOOST_MPL_LIMIT_LIST_SIZE=30
)
if(WIN32)
set(jiminy_DEFINITIONS ${jiminy_DEFINITIONS} _USE_MATH_DEFINES=1 NOMINMAX)
set(jiminy_OPTIONS /EHsc /bigobj /Zc:__cplusplus /permissive- /wd4996 /wd4554)
Expand Down
22 changes: 17 additions & 5 deletions build_tools/cmake/setupPython.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,15 @@ else()
if(CMAKE_VERSION VERSION_LESS "3.12.4")
find_program(Python_EXECUTABLE "python${PYTHON_REQUIRED_VERSION}")
else()
if(CMAKE_VERSION VERSION_LESS "3.14")
set(Python_COMPONENTS_FIND Interpreter)
else()
set(Python_COMPONENTS_FIND Interpreter NumPy)
endif()
if(PYTHON_REQUIRED_VERSION)
find_package(Python ${PYTHON_REQUIRED_VERSION} EXACT COMPONENTS Interpreter)
find_package(Python ${PYTHON_REQUIRED_VERSION} EXACT COMPONENTS ${Python_COMPONENTS_FIND})
else()
find_package(Python COMPONENTS Interpreter)
find_package(Python COMPONENTS ${Python_COMPONENTS_FIND})
endif()
endif()
endif()
Expand Down Expand Up @@ -78,7 +83,7 @@ if("${PYTHON_EXT_SUFFIX}" STREQUAL "")
else()
set(PYTHON_EXT_SUFFIX ".so")
endif()
ENDIF()
endif()

# Include Python headers
execute_process(COMMAND "${Python_EXECUTABLE}" -c
Expand All @@ -93,8 +98,15 @@ if(WIN32)
message(STATUS "Found PythonLibraryDirs: ${PYTHON_ROOT}/libs/")
endif()

# Define NumPy_INCLUDE_DIRS
find_package(NumPy REQUIRED)
# Define Python_NumPy_INCLUDE_DIRS if necessary
if (NOT Python_NumPy_INCLUDE_DIRS)
execute_process(
COMMAND "${Python_EXECUTABLE}" -c
"import numpy; print(numpy.get_include(), end='')\n"
OUTPUT_VARIABLE __numpy_path)
find_path(Python_NumPy_INCLUDE_DIRS numpy/arrayobject.h
HINTS "${__numpy_path}" "${Python_INCLUDE_DIRS}" NO_DEFAULT_PATH)
endif()

# Define BOOST_PYTHON_LIB
find_package(Boost QUIET REQUIRED)
Expand Down
10 changes: 5 additions & 5 deletions build_tools/easy_install_deps_ubuntu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ export DEBIAN_FRONTEND=noninteractive
# Determine if the script is being executed on Ubuntu
if [ -f /etc/lsb-release ]; then
source /etc/lsb-release
if [ "$DISTRIB_ID" != "Ubuntu" ] || ( [ "$DISTRIB_RELEASE" != "18.04" ] && [ "$DISTRIB_RELEASE" != "20.04" ] ) ; then
echo "Not running on Ubuntu 18 or 20. Aborting..."
if [ "$DISTRIB_ID" != "Ubuntu" ] ; then
echo "Not running on Ubuntu. Aborting..."
exit 0
fi
else
echo "Not running on Ubuntu 18 or 20. Aborting..."
echo "Not running on Ubuntu. Aborting..."
exit 0
fi

Expand Down Expand Up @@ -57,8 +57,8 @@ if ! [ -d "/opt/openrobots/lib/${PYTHON_BIN}/site-packages/" ] ; then

# apt-get must be used instead of apt to support wildcard in package name on Ubuntu 20
apt-get install -y --allow-downgrades --allow-unauthenticated \
robotpkg-urdfdom=1.0.3 robotpkg-urdfdom-headers=1.0.4 robotpkg-hpp-fcl=1.7.1 robotpkg-pinocchio=2.5.6 \
robotpkg-py3*-qt5-gepetto-viewer=4.10.1r1 robotpkg-py3*-qt5-gepetto-viewer-corba=5.5.1 robotpkg-py3*-omniorbpy=4.2.4 \
robotpkg-urdfdom-headers=1.0.4 robotpkg-hpp-fcl=1.7.1 robotpkg-pinocchio=2.5.6 \
robotpkg-py3*-qt5-gepetto-viewer=4.10.1r2 robotpkg-py3*-qt5-gepetto-viewer-corba=5.5.1 robotpkg-py3*-omniorbpy=4.2.4 \
robotpkg-py3*-eigenpy=2.6.2 robotpkg-py3*-hpp-fcl=1.7.1 robotpkg-py3*-pinocchio=2.5.6

sudo -H -u $(id -nu "$SUDO_UID") bash -c " \
Expand Down
2 changes: 1 addition & 1 deletion core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ project(${LIBRARY_NAME}_core VERSION ${BUILD_VERSION})

# Find dependencies
find_package(Boost REQUIRED COMPONENTS system filesystem serialization date_time thread)
find_package(urdfdom REQUIRED NO_MODULE NO_CMAKE_SYSTEM_PATH) # It is impossible to specify the version because it is not exported in cmake config files...
find_package(urdfdom REQUIRED NO_MODULE NO_CMAKE_SYSTEM_PATH) # It is impossible to specify the version because it is not exported in cmake config files...
find_package(PkgConfig QUIET) # Using pkgconfig is the only way to get the library version...
if (PkgConfig_FOUND)
pkg_check_modules(_URDFDOM QUIET "urdfdom")
Expand Down
21 changes: 18 additions & 3 deletions core/include/jiminy/core/Macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,23 @@ namespace jiminy
{
std::ostringstream sstr;
using List = int[];
(void)List{0, ( (void)(sstr << args), 0 ) ... };
(void) List{0, (static_cast<void>(sstr << args), 0 ) ... };
return sstr.str();
}

template<size_t FL, size_t PFL>
const char * extractMethodName(char const (&function)[FL],
char const (&prettyFunction)[PFL])
{
using reverse_ptr = std::reverse_iterator<const char*>;
thread_local static char result[PFL];
char const * locFuncName = std::search(prettyFunction,prettyFunction+PFL-1,function,function+FL-1);
char const * locClassName = std::find(reverse_ptr(locFuncName), reverse_ptr(prettyFunction), ' ').base();
char const * endFuncName = std::find(locFuncName,prettyFunction+PFL-1,'(');
std::copy(locClassName, endFuncName, result);
return result;
}

#define STRINGIFY_DETAIL(x) #x
#define STRINGIFY(x) STRINGIFY_DETAIL(x)

Expand All @@ -278,13 +291,15 @@ namespace jiminy
https://solarianprogrammer.com/2019/04/08/c-programming-ansi-escape-codes-windows-macos-linux-terminals/ */

#define PRINT_ERROR(...) \
std::cerr << "In " FILE_LINE ": In " << BOOST_CURRENT_FUNCTION << ":\n\x1b[1;31merror:\x1b[0m " << to_string(__VA_ARGS__) << std::endl
std::cerr << "In " FILE_LINE ": In " << extractMethodName(__func__, BOOST_CURRENT_FUNCTION) \
<< ":\n\x1b[1;31merror:\x1b[0m " << to_string(__VA_ARGS__) << std::endl
#ifdef NDEBUG
#define PRINT_WARNING(...)
#else
#define PRINT_WARNING(...) \
std::cerr << "In " FILE_LINE ": In " << BOOST_CURRENT_FUNCTION << ":\n\x1b[1;93mwarning:\x1b[0m " << to_string(__VA_ARGS__) << std::endl
std::cerr << "In " FILE_LINE ": In " << extractMethodName(__func__, BOOST_CURRENT_FUNCTION) \
<< ":\n\x1b[1;93mwarning:\x1b[0m " << to_string(__VA_ARGS__) << std::endl
#endif
}
Expand Down
Loading

0 comments on commit 8cc93c8

Please sign in to comment.