Parent node's information is lost when performing deepcopy. #388
Conversation
OmegaConf misbehaves with the deepcopy function and the information about the parent (node) is lost. This leads to errors where a variable's value can not be resolved. The error message looks like this - `KeyError: "str interpolation key 'hydra.ax.experiment' not found"`. This commit provides a temporary workaround for the problem, by setting the parent of the copied config as that of the original config. One testcase is also added to check for this bug.
facebook-github-bot
added
the
CLA Signed
tests/test_utils.py
Outdated
| ) -> None: | ||
| # Check if the instantiate method maintains the parent when making a deepcopy | ||
| conf = OmegaConf.create(input_conf) | ||
| orig = copy.deepcopy(conf) |
There was a problem hiding this comment.
I don't think you need to do the deepcopy here.
Instantiate is doing the deepcopy.
There was a problem hiding this comment.
You are also testing passthrough.
just make your test another case of test_class_instantiate.
(You can add additional parameters set to the list there).
There was a problem hiding this comment.
I can get rid of the deepcopy part. Regarding adding extra params to test_class_instantiate, I think it will make it a little messy as we need a nested config and need to call instantiate over a specific node in the config. If that is fine, I can merge the two testcases.
There was a problem hiding this comment.
As long as the test code is the same (and it can be), you can reduce duplication with parametrize.
There was a problem hiding this comment.
Merged the test cases
Two new params are added to the method for testing class instantiation. These are - key_to_get_config (for select which node in the config should be sent to the instantiation method) and kwargs_to_pass (for the key-value args to pass to instantiation method). Now we can merge three of our test cases in a single function.
|
@omry I think this may have accidentally been reverted at some point (it looks like there were big changes to instantiate since this PR), I'm getting this error and master doesn't seem to have the |
|
I think there are tests that should test this. |
Motivation
Parent node's information is lost when performing deepcopy.
OmegaConf misbehaves with the deepcopy function and the information about the parent (node) is lost. This leads to errors where a variable's value can not be resolved. The error message looks like this -
KeyError: "str interpolation key 'hydra.ax.experiment' not found". This commit provides a temporary workaround for the problem, by setting the parent of the copied config as that of the original config.One testcase is also added to check for this bug.
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Added a testcase.