Skip to content

Commit

Permalink
[gym_jiminy/common] Fix cache auto-refresh once again. (#882)
Browse files Browse the repository at this point in the history
* [gym_jiminy/common] Speedup dataset trajectory select.
* [gym_jiminy/common] Fix cache auto-refresh once again. 
* [gym_jiminy/common] Fix 'DeltaQuantity' equality.
  • Loading branch information
duburcqa authored Feb 9, 2025
1 parent 2e0bcb6 commit 256a212
Show file tree
Hide file tree
Showing 9 changed files with 170 additions and 96 deletions.
6 changes: 2 additions & 4 deletions .github/workflows/linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ jobs:
sudo apt update
sudo apt install -y gdb gnupg curl wget build-essential cmake doxygen graphviz texlive-latex-base
"${PYTHON_EXECUTABLE}" -m pip install setuptools wheel "pip>=20.3"
"${PYTHON_EXECUTABLE}" -m pip install setuptools wheel "pip>=20.3,<25.1"
git config --global advice.detachedHead false
- name: Install pre-compiled binaries for additional gym-jiminy dependencies
Expand Down Expand Up @@ -140,9 +140,7 @@ jobs:
if: matrix.BUILD_TYPE != 'Debug' && matrix.PYTHON_VERSION == '3.10'
run: |
# Generate stubs
# FIXME: stubgen does not work with Numpy 2.X without option `--no-analysis`
# (see https://github.com/python/mypy/issues/17396)
stubgen -p jiminy_py -o ${RootDir}/build/pypi/jiminy_py/src --no-analysis
stubgen -p jiminy_py -o ${RootDir}/build/pypi/jiminy_py/src
"${PYTHON_EXECUTABLE}" "${RootDir}/build_tools/stubgen.py" \
-o ${RootDir}/build/stubs --ignore-invalid=all jiminy_py
cp ${RootDir}/build/stubs/jiminy_py-stubs/core/__init__.pyi \
Expand Down
6 changes: 2 additions & 4 deletions .github/workflows/macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ jobs:
echo "JIMINY_PANDA3D_FORCE_TINYDISPLAY=" >> $GITHUB_ENV
fi
"${PYTHON_EXECUTABLE}" -m pip install setuptools wheel "pip>=20.3"
"${PYTHON_EXECUTABLE}" -m pip install setuptools wheel "pip>=20.3,<25.1"
"${PYTHON_EXECUTABLE}" -m pip install delocate twine
- name: Install pre-compiled binaries for additional gym-jiminy dependencies
run: |
Expand Down Expand Up @@ -142,9 +142,7 @@ jobs:
# Generate stubs
if [[ "${{ matrix.BUILD_TYPE }}" != 'Debug' && "${{ matrix.OS }}" != 'macos-13' ]] ; then
# FIXME: stubgen does not work with Numpy 2.X without option `--no-analysis`
# (see https://github.com/python/mypy/issues/17396)
stubgen -p jiminy_py -o ${RootDir}/build/pypi/jiminy_py/src --no-analysis
stubgen -p jiminy_py -o ${RootDir}/build/pypi/jiminy_py/src
# FIXME: Python 3.10 and Python 3.11 crashes when generating stubs without any backtrace...
if [[ "${{ matrix.PYTHON_VERSION }}" != '3.10' && "${{ matrix.PYTHON_VERSION }}" != '3.11' ]] ; then
# lldb --batch -o "settings set target.process.stop-on-exec false" \
Expand Down
6 changes: 2 additions & 4 deletions .github/workflows/manylinux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ jobs:
echo "RootDir=${GITHUB_WORKSPACE}" >> $GITHUB_ENV
echo "InstallDir=${GITHUB_WORKSPACE}/install" >> $GITHUB_ENV
"${PYTHON_EXECUTABLE}" -m pip install setuptools wheel "pip>=20.3"
"${PYTHON_EXECUTABLE}" -m pip install setuptools wheel "pip>=20.3,<25.1"
"${PYTHON_EXECUTABLE}" -m pip install twine cmake
- name: Install latest numpy version at build-time for run-time binary compatibility
run: |
Expand Down Expand Up @@ -112,9 +112,7 @@ jobs:
export LD_LIBRARY_PATH="$InstallDir/lib:$InstallDir/lib64:/usr/local/lib"
# Generate stubs
# FIXME: stubgen does not work with Numpy 2.X without option `--no-analysis`
# (see https://github.com/python/mypy/issues/17396)
stubgen -p jiminy_py -o $RootDir/build/pypi/jiminy_py/src --no-analysis
stubgen -p jiminy_py -o $RootDir/build/pypi/jiminy_py/src
"${PYTHON_EXECUTABLE}" "$RootDir/build_tools/stubgen.py" \
-o $RootDir/build/stubs --ignore-invalid=all jiminy_py
\cp $RootDir/build/stubs/jiminy_py-stubs/core/__init__.pyi \
Expand Down
6 changes: 2 additions & 4 deletions .github/workflows/win.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ jobs:
- name: Setup minimal build environment
run: |
git config --global advice.detachedHead false
python -m pip install setuptools wheel "pip>=20.3"
python -m pip install setuptools wheel "pip>=20.3,<25.1"
python -m pip install pefile machomachomangler
- name: Install pre-compiled binaries for additional gym-jiminy dependencies
run: |
Expand Down Expand Up @@ -123,9 +123,7 @@ jobs:
${env:Path} += ";$InstallDir/lib"
# Generate stubs
# FIXME: stubgen does not work with Numpy 2.X without option `--no-analysis`
# (see https://github.com/python/mypy/issues/17396)
stubgen -p jiminy_py -o $RootDir/build/pypi/jiminy_py/src --no-analysis
stubgen -p jiminy_py -o $RootDir/build/pypi/jiminy_py/src
python "$RootDir/build_tools/stubgen.py" `
-o $RootDir/build/stubs --ignore-invalid=all jiminy_py
Copy-Item -Force -Path "$RootDir/build/stubs/jiminy_py-stubs/core/__init__.pyi" `
Expand Down
2 changes: 1 addition & 1 deletion build_tools/easy_install_deps_ubuntu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ echo "-- Python writable site-packages: ${PYTHON_SITELIB}"
# Install Python 3 standard utilities
apt update && \
apt install -y python3-pip && \
${SUDO_CMD} python3 -m pip install setuptools wheel "pip>=20.3" && \
${SUDO_CMD} python3 -m pip install setuptools wheel "pip>=20.3,<25.1" && \
${SUDO_CMD} python3 -m pip install "numpy>=1.24" "numba>=0.54.0"

# Install standard linux utilities
Expand Down
93 changes: 63 additions & 30 deletions python/gym_jiminy/common/gym_jiminy/common/bases/quantities.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,14 @@ def get(self) -> ValueT:

# Initialize quantity if not already done manually
if not owner._is_initialized:
owner.initialize()
try:
owner.initialize()
except Exception:
# Revert initialization of this quantity as it failed
owner.reset(reset_tracking=False,
ignore_requirements=True,
ignore_others=True)
raise
assert owner._is_initialized

# Get first owning quantity systematically
Expand Down Expand Up @@ -577,7 +584,9 @@ def get(self) -> ValueT:

def reset(self,
reset_tracking: bool = False,
*, ignore_other_instances: bool = False) -> None:
*,
ignore_requirements: bool = False,
ignore_others: bool = False) -> None:
"""Consider that the quantity must be re-initialized before being
evaluated once again.
Expand All @@ -595,9 +604,11 @@ def reset(self,
:param reset_tracking: Do not consider this quantity as active anymore
until the `get` method gets called once again.
Optional: False by default.
:param ignore_other_instances:
Whether to skip reset of intermediary quantities as well as any
shared cache co-owner quantity instances.
:param ignore_requirements:
Whether to skip reset reset of intermediary quantities.
Optional: False by default.
:param ignore_others:
Whether to ignore any shared cache co-owner quantity instances.
Optional: False by default.
"""
# Make sure that auto-refresh can be honored
Expand All @@ -609,9 +620,14 @@ def reset(self,
# Reset all requirements first.
# This is necessary to avoid auto-refreshing quantities with deprecated
# cache if enabled.
if not ignore_other_instances:
if not ignore_requirements:
for quantity in self.requirements.values():
quantity.reset(reset_tracking, ignore_other_instances=False)
quantity.reset(reset_tracking,
ignore_requirements=False,
ignore_others=ignore_others)

# No longer consider this exact instance as initialized
self._is_initialized = False

# Skip reset if dynamic computation graph update is not allowed
if self.env.is_simulation_running and not self.allow_update_graph:
Expand All @@ -621,27 +637,26 @@ def reset(self,
if reset_tracking:
self._is_active = False

# No longer consider this exact instance as initialized
self._is_initialized = False

# More work must to be done if this quantity has a shared cache that
# has not been completely reset yet.
if self.has_cache and self.cache.sm_state is not _IS_RESET:
# Reset shared cache state machine first, to avoid triggering reset
# propagation to all identical quantities.
self.cache.reset(
ignore_auto_refresh=True, reset_state_machine=True)
self.cache.reset(ignore_auto_refresh=True,
reset_state_machine=True)

# Reset all identical quantities except itself since already done
for owner in self.cache.owners:
if owner is not self:
owner.reset(reset_tracking=reset_tracking,
ignore_other_instances=True)
if not ignore_others:
for owner in self.cache.owners:
if owner is not self:
owner.reset(reset_tracking=reset_tracking,
ignore_requirements=True,
ignore_others=True)

# Reset shared cache afterward with auto-refresh enabled if needed
if self.env.is_simulation_running:
self.cache.reset(
ignore_auto_refresh=False, reset_state_machine=False)
self.cache.reset(ignore_auto_refresh=False,
reset_state_machine=False)

def initialize(self) -> None:
"""Initialize internal buffers.
Expand Down Expand Up @@ -793,6 +808,11 @@ def initialize(self) -> None:
try:
self.state.initialize()
except RuntimeError:
# Revert state initialization
self.state.reset(reset_tracking=False,
ignore_requirements=True,
ignore_others=True)

# It may have failed because no simulation running, which may be
# problematic but not blocking at this point. Just checking that
# the pinocchio model has been properly initialized.
Expand Down Expand Up @@ -847,9 +867,9 @@ def fun_safe(self: InterfaceQuantity, *args: Any, **kwargs: Any) -> None:

@dataclass(unsafe_hash=True)
class DatasetTrajectoryQuantity(InterfaceQuantity[State]):
"""This class manages a database of trajectories.
"""This class manages a dataset of trajectories.
The database is empty by default. Trajectories must be added or discarded
The dataset is empty by default. Trajectories must be added or discarded
manually. Only one trajectory can be selected at once. Once a trajectory
has been selecting, its state at the current simulation can be easily
retrieved.
Expand Down Expand Up @@ -1129,13 +1149,13 @@ def clear(self) -> None:
self._registry.clear()

@sync
def select(self, name: str) -> None:
"""Select an existing trajectory from the database shared synchronized
all managed quantities.
def _select(self, name: str) -> None:
"""Select an existing trajectory from the dataset shared amongst all
managed quantities without resetting the quantity itself at this point.
.. note::
There is no way to select a different reference trajectory for
individual quantities at the time being.
.. warning::
This method is used internally by `select` method. It is not meant
o be called manually.
:param name: Name of the trajectory to select.
"""
Expand All @@ -1144,15 +1164,28 @@ def select(self, name: str) -> None:
raise ValueError("Cannot select trajectory on a empty dataset.")

# Select the desired trajectory for all identical instances
self._trajectory_mode_pair = self._registry[name]
trajectory, _ = self._trajectory_mode_pair = self._registry[name]
self._name = name

# Update the absolute time ratio
time_start, time_end = self.trajectory.time_interval
time_start, time_end = trajectory.time_interval
time_delta = time_end - time_start
self._time_offset = time_start + self.time_offset_ratio * time_delta
self._time_offset = time_start + self._time_offset_ratio * time_delta

def select(self, name: str) -> None:
"""Select an existing trajectory from the dataset shared amongst all
managed quantities, then reset the quantity itself.
.. note::
There is no way to select a different reference trajectory for
individual quantities at the time being.
:param name: Name of the trajectory to select.
"""
# Update the selected trajectory for all the instances at once
self._select(name)

# Un-initialize quantity when the selected trajectory changes
# Un-initialize all instances of this quantity
self.reset(reset_tracking=False)

@sync
Expand Down
Loading

0 comments on commit 256a212

Please sign in to comment.