Unverified Commit 394a1629 authored by Özhan Özen's avatar Özhan Özen Committed by GitHub

Modifies `update_class_from_dict()` to wholesale replace flat Iterables (#2511)

# Description

Currently, `update_class_from_dict()` does not allow updating the
configclass Iterable elements with Hydra, when the provided Iterable
length differs from the default value. Such a feature is nevertheless
needed when changing the network layer depth on the go, e.g., while
using learning libraries that utilize configclass (see #2456 for
details).

This PR modifies `update_class_from_dict()` such that if an element is a
flat Iterable (e.g., flat list), it is replaced wholesale, without
checking the lengths. Moreover, the PR modifies the robustness of the
function against a few edge cases that might break the execution, and
adds comments to make it easier to follow the logic flow.

Note: I left the changelog entry as `[Unreleased]` until a green light
is given.

Fixes #2456.

## Type of change

- Bug fix (non-breaking change which fixes an issue)

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [x] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

---------
Signed-off-by: 's avatarKelly Guo <kellyg@nvidia.com>
Signed-off-by: 's avatarKelly Guo <kellyguo123@hotmail.com>
Co-authored-by: 's avatarKelly Guo <kellyg@nvidia.com>
Co-authored-by: 's avatarKelly Guo <kellyguo123@hotmail.com>
parent fa8612ff
[package] [package]
# Note: Semantic Versioning is used: https://semver.org/ # Note: Semantic Versioning is used: https://semver.org/
version = "0.40.9" version = "0.40.10"
# Description # Description
title = "Isaac Lab framework for Robot Learning" title = "Isaac Lab framework for Robot Learning"
......
Changelog Changelog
--------- ---------
0.40.10 (2025-06-25)
~~~~~~~~~~~~~~~~~~~~
Fixed
^^^^^
* Fixed :meth:`omni.isaac.lab.utils.dict.update_class_from_dict` preventing setting flat Iterables with different lengths.
0.40.9 (2025-06-25) 0.40.9 (2025-06-25)
~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~
......
...@@ -9,7 +9,7 @@ import collections.abc ...@@ -9,7 +9,7 @@ import collections.abc
import hashlib import hashlib
import json import json
import torch import torch
from collections.abc import Iterable, Mapping from collections.abc import Iterable, Mapping, Sized
from typing import Any from typing import Any
from .array import TENSOR_TYPE_CONVERSIONS, TENSOR_TYPES from .array import TENSOR_TYPE_CONVERSIONS, TENSOR_TYPES
...@@ -90,47 +90,79 @@ def update_class_from_dict(obj, data: dict[str, Any], _ns: str = "") -> None: ...@@ -90,47 +90,79 @@ def update_class_from_dict(obj, data: dict[str, Any], _ns: str = "") -> None:
for key, value in data.items(): for key, value in data.items():
# key_ns is the full namespace of the key # key_ns is the full namespace of the key
key_ns = _ns + "/" + key key_ns = _ns + "/" + key
# check if key is present in the object
if hasattr(obj, key) or isinstance(obj, dict): # -- A) if key is present in the object ------------------------------------
if hasattr(obj, key) or (isinstance(obj, dict) and key in obj):
obj_mem = obj[key] if isinstance(obj, dict) else getattr(obj, key) obj_mem = obj[key] if isinstance(obj, dict) else getattr(obj, key)
# -- 1) nested mapping → recurse ---------------------------
if isinstance(value, Mapping): if isinstance(value, Mapping):
# recursively call if it is a dictionary # recursively call if it is a dictionary
update_class_from_dict(obj_mem, value, _ns=key_ns) update_class_from_dict(obj_mem, value, _ns=key_ns)
continue continue
# -- 2) iterable (list / tuple / etc.) ---------------------
if isinstance(value, Iterable) and not isinstance(value, str): if isinstance(value, Iterable) and not isinstance(value, str):
# check length of value to be safe
if len(obj_mem) != len(value) and obj_mem is not None: # ---- 2a) flat iterable → replace wholesale ----------
if all(not isinstance(el, Mapping) for el in value):
out_val = tuple(value) if isinstance(obj_mem, tuple) else value
if isinstance(obj, dict):
obj[key] = out_val
else:
setattr(obj, key, out_val)
continue
# ---- 2b) existing value is None → abort -------------
if obj_mem is None:
raise ValueError(
f"[Config]: Cannot merge list under namespace: {key_ns} because the existing value is None."
)
# ---- 2c) length mismatch → abort -------------------
if isinstance(obj_mem, Sized) and isinstance(value, Sized) and len(obj_mem) != len(value):
raise ValueError( raise ValueError(
f"[Config]: Incorrect length under namespace: {key_ns}." f"[Config]: Incorrect length under namespace: {key_ns}."
f" Expected: {len(obj_mem)}, Received: {len(value)}." f" Expected: {len(obj_mem)}, Received: {len(value)}."
) )
# ---- 2d) keep tuple/list parity & recurse ----------
if isinstance(obj_mem, tuple): if isinstance(obj_mem, tuple):
value = tuple(value) value = tuple(value)
else: else:
set_obj = True set_obj = True
# recursively call if iterable contains dictionaries # recursively call if iterable contains Mappings
for i in range(len(obj_mem)): for i in range(len(obj_mem)):
if isinstance(value[i], dict): if isinstance(value[i], Mapping):
update_class_from_dict(obj_mem[i], value[i], _ns=key_ns) update_class_from_dict(obj_mem[i], value[i], _ns=key_ns)
set_obj = False set_obj = False
# do not set value to obj, otherwise it overwrites the cfg class with the dict # do not set value to obj, otherwise it overwrites the cfg class with the dict
if not set_obj: if not set_obj:
continue continue
# -- 3) callable attribute → resolve string --------------
elif callable(obj_mem): elif callable(obj_mem):
# update function name # update function name
value = string_to_callable(value) value = string_to_callable(value)
elif isinstance(value, type(obj_mem)) or value is None:
# -- 4) simple scalar / explicit None ---------------------
elif value is None or isinstance(value, type(obj_mem)):
pass pass
# -- 5) type mismatch → abort -----------------------------
else: else:
raise ValueError( raise ValueError(
f"[Config]: Incorrect type under namespace: {key_ns}." f"[Config]: Incorrect type under namespace: {key_ns}."
f" Expected: {type(obj_mem)}, Received: {type(value)}." f" Expected: {type(obj_mem)}, Received: {type(value)}."
) )
# set value
# -- 6) final assignment ---------------------------------
if isinstance(obj, dict): if isinstance(obj, dict):
obj[key] = value obj[key] = value
else: else:
setattr(obj, key, value) setattr(obj, key, value)
# -- B) if key is not present ------------------------------------
else: else:
raise KeyError(f"[Config]: Key not found under namespace: {key_ns}.") raise KeyError(f"[Config]: Key not found under namespace: {key_ns}.")
......
...@@ -643,6 +643,28 @@ def test_config_update_nested_dict(): ...@@ -643,6 +643,28 @@ def test_config_update_nested_dict():
assert isinstance(cfg.list_1[1].viewer, ViewerCfg) assert isinstance(cfg.list_1[1].viewer, ViewerCfg)
def test_config_update_different_iterable_lengths():
"""Iterables are whole replaced, even if their lengths are different."""
# original cfg has length-6 tuple and list
cfg = RobotDefaultStateCfg()
assert cfg.dof_pos == (0.0, 0.0, 0.0, 0.0, 0.0, 0.0)
assert cfg.dof_vel == [0.0, 0.0, 0.0, 0.0, 0.0, 1.0]
# patch uses different lengths
patch = {
"dof_pos": (1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0), # longer tuple
"dof_vel": [9.0, 8.0, 7.0], # shorter list
}
# should not raise
update_class_from_dict(cfg, patch)
# whole sequences are replaced
assert cfg.dof_pos == (1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0)
assert cfg.dof_vel == [9.0, 8.0, 7.0]
def test_config_update_dict_using_internal(): def test_config_update_dict_using_internal():
"""Test updating configclass from a dictionary using configclass method.""" """Test updating configclass from a dictionary using configclass method."""
cfg = BasicDemoCfg() cfg = BasicDemoCfg()
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment