Skip to content

Commit

Permalink
Deepcopy full config in instantiate to avoid unexpected side effects
Browse files Browse the repository at this point in the history
  • Loading branch information
jesszzzz committed Jan 3, 2025
1 parent 0ede840 commit 7e6066a
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
27 changes: 23 additions & 4 deletions hydra/_internal/instantiate/_instantiate2.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,25 @@ def _resolve_target(
return target


def _deep_copy_full_config(subconfig: Any) -> Any:
"""Deep copy full config from root to leaf and return the copied subconfig"""
if not OmegaConf.is_config(subconfig):
return copy.deepcopy(subconfig)

full_key = subconfig._get_full_key(None)
if full_key:
full_config_copy = copy.deepcopy(subconfig._get_root())
if OmegaConf.is_list(subconfig._get_parent()):
# OmegaConf has a bug where _get_full_key doesn't add [] if the parent
# is a list, eg. instead of foo[0], it'll return foo0
index = subconfig._key()
full_key = full_key[:-len(str(index))] + f"[{index}]"
_root, _last_key, node = full_config_copy._select_impl(full_key, True, False)
return node
else:
return copy.deepcopy(subconfig)


def instantiate(config: Any, *args: Any, **kwargs: Any) -> Any:
"""
:param config: An config object describing what to call and what params to use.
Expand Down Expand Up @@ -207,11 +226,11 @@ def instantiate(config: Any, *args: Any, **kwargs: Any) -> Any:

if OmegaConf.is_dict(config):
# Finalize config (convert targets to strings, merge with kwargs)
config_copy = copy.deepcopy(config)
# Create full copy to avoid mutating original
config_copy = _deep_copy_full_config(config)
config_copy._set_flag(
flags=["allow_objects", "struct", "readonly"], values=[True, False, False]
)
config_copy._set_parent(config._get_parent())
config = config_copy

if kwargs:
Expand All @@ -228,11 +247,11 @@ def instantiate(config: Any, *args: Any, **kwargs: Any) -> Any:
)
elif OmegaConf.is_list(config):
# Finalize config (convert targets to strings, merge with kwargs)
config_copy = copy.deepcopy(config)
# Create full copy to avoid mutating original
config_copy = _deep_copy_full_config(config)
config_copy._set_flag(
flags=["allow_objects", "struct", "readonly"], values=[True, False, False]
)
config_copy._set_parent(config._get_parent())
config = config_copy

OmegaConf.resolve(config)
Expand Down
1 change: 1 addition & 0 deletions news/3001.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix unexpected resolution side-effect that caused modifications to the input config parent in `hydra.utils.instantiate`

0 comments on commit 7e6066a

Please sign in to comment.