Modifies update_class_from_dict() to wholesale replace flat Iterables

[ozhanozen]

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

• I have run the pre-commit checks 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
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[Mayankm96]

Agreed. Faced a similar issue. Can you also add a unit test for this change to make sure we maintain the desired behavior?

Also wondering: flat Iterable (e.g., flat list), it is replaced wholesale, without checking the lengths -- I don't think there are situations where we don't want to do this change. Initially, it must have been to keep it safe while updating lists/tuples (for instance, when specifying ranges for parameters) and not let users write wrong length of arrays. But I am also in for just simplifying the whole thing and throwing a warning instead of a error while updating a list/tuple of different lengths. This should simplify the code too.

[ozhanozen]

Agreed. Faced a similar issue. Can you also add a unit test for this change to make sure we maintain the desired behavior?

I have added the unit test case (i.e., test_config_update_different_iterable_lengths) inside test_configclass.py.

Also wondering: flat Iterable (e.g., flat list), it is replaced wholesale, without checking the lengths -- I don't think there are situations where we don't want to do this change. Initially, it must have been to keep it safe while updating lists/tuples (for instance, when specifying ranges for parameters) and not let users write wrong length of arrays. But I am also in for just simplifying the whole thing and throwing a warning instead of a error while updating a list/tuple of different lengths. This should simplify the code too.

Do you mean we should replace the Iterable elements wholesale, wherever they are flat Iterable or not?

This creates an issue when the list has a subdictionary (e.g., main_dict -> list -> sub_dict). Actually, there is a specific test case for such a situation (test_config_update_nested_dict inside test_configclass.py), which fails if the list containing the sub_dict is replaced. I thought maybe this is an edge-case which could be removed, but then I had associated errors while running trainings, so I guess it has to be covered.

Let me know if you meant something else or you have other feedback.