Fixes configclass shared references to keep compound objects independent...

Fixes configclass shared references to keep compound objects independent across subclass instances (#528)

# Description

There is currently an issue with `configclass.py`'s function
`_return_f`, which produces `default_factory` functions for
`configclass` classes.

`configclass` instances which have member variables that are compound
objects, (such as `BaseEnvCfg` and its member `SimulationCfg`) currently
share a reference to a common members (in the example of `BaseEnvCfg`,
its inheriting classes would share a common reference to `self.sim.dt`)

When such classes are inherited, compound objects which have their
member variables changed in the __post_init__ function will be changed
for all subclass instances. I have solved this by changing the non-field
return to be a `deepcopy`

The issue was observed in orbit.eval_sim and was reported this way on
the Institute's JIRA:
> EvalSim will hold the Environment parameters that are set from the
first environment that is loaded in unless it is otherwise explicitly
overridden in the post init or reset EnvCfg member fields by the next
environment.
> 
> Example: 
>
>If EnvironmentCfg A gets loaded first and sets sim.dt = 0.1 followed by
loading EnvironmentCfg B, then Environment B will get the sim.dt set in
Environment A if it does not explicitly set sim.dt in the post_init. Any
fields defined inside BaseEnvCfg that are not redefined inside the Cfg
inheriting BaseEnvCfg will take after the configuration set in the first
environment that gets loaded.

I have added a condition to the test
`test_configclass.test_config_inheritance` which tests this value. Those
wishing to demonstrate that this issue exists can simply change the line
in `configclass.py` back to an assignment and observe the test failing.

## Type of change

<!-- As you go through the list, delete the ones that are not
applicable. -->

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

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./orbit.sh --format`
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] I have run all the tests with `./orbit.sh --test` and they pass
- [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: Hunter Hansen <50837800+hhansen-bdai@users.noreply.github.com>
Co-authored-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com>

source/extensions/omni.isaac.orbit/config/extension.toml

 [package]
 
 # Note: Semantic Versioning is used: https://semver.org/
-version = "0.16.3"
+version = "0.16.4"
 
 # Description
 title = "ORBIT framework for Robot Learning"

source/extensions/omni.isaac.orbit/docs/CHANGELOG.rst

 Changelog
 ---------
 
+0.16.4 (2024-05-15)
+~~~~~~~~~~~~~~~~~~~
+
+Fixed
+^^^^^
+
+* Fixed compound classes being directly assigned in ``default_factory`` generator method :meth:`omni.isaac.orbit.utils.configclass._return_f`,
+  which resulted in shared references such that modifications to compound objects were reflected across all instances generated from the same ``default_factory`` method.
+
+
 0.16.3 (2024-05-13)
 ~~~~~~~~~~~~~~~~~~~
 

source/extensions/omni.isaac.orbit/omni/isaac/orbit/utils/configclass.py

@@ -415,6 +415,6 @@ def _return_f(f: Any) -> Callable[[], Any]:
             else:
                 return f.default_factory
         else:
-            return f
+            return deepcopy(f)
 
     return _wrap

source/extensions/omni.isaac.orbit/test/utils/test_configclass.py

@@ -168,13 +168,14 @@ class ParentDemoCfg:
     a: int = MISSING  # add new missing field
     b = 2  # type annotation missing on purpose
     c: RobotDefaultStateCfg = MISSING  # add new missing field
+    m: RobotDefaultStateCfg = RobotDefaultStateCfg()  # Add class type with defaults
     j: list[str] = MISSING  # add new missing field
     i: list[str] = MISSING  # add new missing field
     func: Callable = MISSING  # add new missing field
 
 
 @configclass
-class ChildDemoCfg(ParentDemoCfg):
+class ChildADemoCfg(ParentDemoCfg):
     """Dummy child configuration with missing fields."""
 
     func = dummy_function1  # set default value for missing field
@@ -189,11 +190,24 @@ class ChildDemoCfg(ParentDemoCfg):
 
     def __post_init__(self):
         self.b = 3  # change value of existing field
+        self.m.rot = (2.0, 0.0, 0.0, 0.0)  # change value of default
         self.i = ["a", "b"]  # change value of existing field
 
 
 @configclass
-class ChildChildDemoCfg(ChildDemoCfg):
+class ChildBDemoCfg(ParentDemoCfg):
+    """Dummy child configuration to test inheritance across instances."""
+
+    a = 100  # set default value for missing field
+    j = ["3", "4"]  # set default value for missing field
+
+    def __post_init__(self):
+        self.b = 8  # change value of existing field
+        self.i = ["1", "2"]  # change value of existing field
+
+
+@configclass
+class ChildChildDemoCfg(ChildADemoCfg):
     """Dummy child configuration with missing fields."""
 
     func_2 = dummy_function2
@@ -613,16 +627,51 @@ class TestConfigClass(unittest.TestCase):
     def test_config_inheritance(self):
         """Tests that inheritance works properly."""
         # check variables
-        cfg = ChildDemoCfg(a=20, d=3, e=ViewerCfg(), j=["c", "d"])
+        cfg_a = ChildADemoCfg(a=20, d=3, e=ViewerCfg(), j=["c", "d"])
 
-        self.assertEqual(cfg.func, dummy_function1)
-        self.assertEqual(cfg.a, 20)
-        self.assertEqual(cfg.d, 3)
-        self.assertEqual(cfg.j, ["c", "d"])
+        self.assertEqual(cfg_a.func, dummy_function1)
+        self.assertEqual(cfg_a.a, 20)
+        self.assertEqual(cfg_a.d, 3)
+        self.assertEqual(cfg_a.j, ["c", "d"])
 
         # check post init
-        self.assertEqual(cfg.b, 3)
-        self.assertEqual(cfg.i, ["a", "b"])
+        self.assertEqual(cfg_a.b, 3)
+        self.assertEqual(cfg_a.i, ["a", "b"])
+        self.assertEqual(cfg_a.m.rot, (2.0, 0.0, 0.0, 0.0))
+
+    def test_config_inheritance_independence(self):
+        """Tests that subclass instantions have fully unique members,
+        rather than references to members of the parent class"""
+        # instantiate two classes which inherit from a shared parent,
+        # but which will differently modify their members in their
+        # __init__ and  __post_init__
+        cfg_a = ChildADemoCfg()
+        cfg_b = ChildBDemoCfg()
+
+        # Test various combinations of initialization
+        # and defaults across inherited members in
+        # instances to verify independence between the subclasses
+        self.assertIsInstance(cfg_a.a, type(MISSING))
+        self.assertEqual(cfg_b.a, 100)
+        self.assertEqual(cfg_a.b, 3)
+        self.assertEqual(cfg_b.b, 8)
+        self.assertEqual(cfg_a.c, RobotDefaultStateCfg())
+        self.assertIsInstance(cfg_b.c, type(MISSING))
+        self.assertEqual(cfg_a.m.rot, (2.0, 0.0, 0.0, 0.0))
+        self.assertEqual(cfg_b.m.rot, (1.0, 0.0, 0.0, 0.0))
+        self.assertIsInstance(cfg_a.j, type(MISSING))
+        self.assertEqual(cfg_b.j, ["3", "4"])
+        self.assertEqual(cfg_a.i, ["a", "b"])
+        self.assertEqual(cfg_b.i, ["1", "2"])
+        self.assertEqual(cfg_a.func, dummy_function1)
+        self.assertIsInstance(cfg_b.func, type(MISSING))
+
+        # Explicitly assert that members are not the same object
+        # for different levels and kinds of data types
+        self.assertIsNot(cfg_a.m, cfg_b.m)
+        self.assertIsNot(cfg_a.m.rot, cfg_b.m.rot)
+        self.assertIsNot(cfg_a.i, cfg_b.i)
+        self.assertIsNot(cfg_a.b, cfg_b.b)
 
     def test_config_double_inheritance(self):
         """Tests that inheritance works properly when inheriting twice."""
@@ -682,7 +731,7 @@ class TestConfigClass(unittest.TestCase):
         filename = os.path.join(dirname, "output", "configclass", "test_config.yaml")
 
         # create config
-        cfg = ChildDemoCfg(a=20, d=3, e=ViewerCfg(), j=["c", "d"])
+        cfg = ChildADemoCfg(a=20, d=3, e=ViewerCfg(), j=["c", "d"])
 
         # save config
         dump_yaml(filename, cfg)