config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
# Owner(s): ["module: unknown"]
|
2024-11-01 16:26:02 +00:00
|
|
|
import os
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
import pickle
|
2024-11-20 00:16:27 +00:00
|
|
|
from unittest.mock import patch
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
|
2024-11-01 16:26:02 +00:00
|
|
|
|
|
|
|
|
os.environ["ENV_TRUE"] = "1"
|
|
|
|
|
os.environ["ENV_FALSE"] = "0"
|
2025-01-29 23:19:05 +00:00
|
|
|
os.environ["ENV_STR"] = "1234"
|
|
|
|
|
os.environ["ENV_STR_EMPTY"] = ""
|
2024-11-01 16:26:02 +00:00
|
|
|
|
2024-11-07 03:49:07 +00:00
|
|
|
from typing import Optional
|
|
|
|
|
|
2024-12-14 17:24:12 +00:00
|
|
|
from torch.testing._internal import (
|
|
|
|
|
fake_config_module as config,
|
|
|
|
|
fake_config_module2 as config2,
|
[minifier] Fix config generator for callables (#144518)
Summary:
When config contains callables, the current configs generated cannot be run:
```
torch._dynamo.config.reorderable_logging_functions = {<built-in function print>, <function warning at 0x7f774c595630>, <function log at 0x7f774c595870>, <function error at 0x7f774c595510>, <function info at 0x7f774c595750>, <built-in function warn>, <function exception at 0x7f774c5955a0>, <function debug at 0x7f774c5957e0>, <function critical at 0x7f774c5953f0>}
```
We fix the config to generate the right string, so the config is runnable, like below
```
import logging
import warnings
torch._dynamo.config.reorderable_logging_functions = { warnings.warn, logging.warn, print }
```
Test Plan:
```
buck2 run 'fbcode//mode/dev-nosan' fbcode//caffe2/test:utils -- -r test_codegen_config
```
Differential Revision: D67998703
Pull Request resolved: https://github.com/pytorch/pytorch/pull/144518
Approved by: https://github.com/desertfire
2025-01-14 17:18:13 +00:00
|
|
|
fake_config_module3 as config3,
|
2024-12-14 17:24:12 +00:00
|
|
|
)
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
from torch.testing._internal.common_utils import run_tests, TestCase
|
2025-02-05 19:40:10 +00:00
|
|
|
from torch.utils._config_module import _ConfigEntry, _UNSET_SENTINEL, Config
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
|
|
|
|
|
|
|
|
|
|
class TestConfigModule(TestCase):
|
2024-12-04 23:08:30 +00:00
|
|
|
def tearDown(self):
|
|
|
|
|
# Config changes get persisted between test cases
|
|
|
|
|
for k in config._config:
|
|
|
|
|
config._config[k].user_override = _UNSET_SENTINEL
|
|
|
|
|
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
def test_base_value_loading(self):
|
|
|
|
|
self.assertTrue(config.e_bool)
|
|
|
|
|
self.assertTrue(config.nested.e_bool)
|
2024-11-07 03:49:07 +00:00
|
|
|
self.assertTrue(config.e_optional)
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
self.assertEqual(config.e_int, 1)
|
|
|
|
|
self.assertEqual(config.e_float, 1.0)
|
|
|
|
|
self.assertEqual(config.e_string, "string")
|
|
|
|
|
self.assertEqual(config.e_list, [1])
|
|
|
|
|
self.assertEqual(config.e_set, {1})
|
|
|
|
|
self.assertEqual(config.e_tuple, (1,))
|
|
|
|
|
self.assertEqual(config.e_dict, {1: 2})
|
|
|
|
|
self.assertEqual(config.e_none, None)
|
|
|
|
|
with self.assertRaises(
|
|
|
|
|
AttributeError, msg="fake_config_module.does_not_exist does not exist"
|
|
|
|
|
):
|
|
|
|
|
config.does_not_exist
|
|
|
|
|
|
2024-11-07 03:49:07 +00:00
|
|
|
def test_type_loading(self):
|
|
|
|
|
self.assertEqual(config.get_type("e_optional"), Optional[bool])
|
|
|
|
|
self.assertEqual(config.get_type("e_none"), Optional[bool])
|
|
|
|
|
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
def test_overrides(self):
|
|
|
|
|
config.e_bool = False
|
|
|
|
|
self.assertFalse(config.e_bool)
|
|
|
|
|
config.nested.e_bool = False
|
|
|
|
|
self.assertFalse(config.nested.e_bool)
|
|
|
|
|
config.e_int = 2
|
|
|
|
|
self.assertEqual(config.e_int, 2)
|
|
|
|
|
config.e_float = 2.0
|
|
|
|
|
self.assertEqual(config.e_float, 2.0)
|
|
|
|
|
config.e_string = "string2"
|
|
|
|
|
self.assertEqual(config.e_string, "string2")
|
|
|
|
|
config.e_list = [2]
|
|
|
|
|
self.assertEqual(config.e_list, [2])
|
|
|
|
|
config.e_set = {2}
|
|
|
|
|
self.assertEqual(config.e_set, {2})
|
|
|
|
|
config.e_tuple = (2,)
|
|
|
|
|
self.assertEqual(config.e_tuple, (2,))
|
|
|
|
|
config.e_dict = {2: 3}
|
|
|
|
|
self.assertEqual(config.e_dict, {2: 3})
|
|
|
|
|
config.e_none = "not none"
|
|
|
|
|
self.assertEqual(config.e_none, "not none")
|
|
|
|
|
config.e_none = None
|
|
|
|
|
self.assertEqual(config.e_none, None)
|
2024-11-07 03:49:07 +00:00
|
|
|
config.e_optional = None
|
|
|
|
|
self.assertEqual(config.e_optional, None)
|
|
|
|
|
config.e_optional = False
|
|
|
|
|
self.assertEqual(config.e_optional, False)
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
with self.assertRaises(
|
|
|
|
|
AttributeError, msg="fake_config_module.does_not_exist does not exist"
|
|
|
|
|
):
|
|
|
|
|
config.does_not_exist = 0
|
|
|
|
|
|
2024-10-29 20:38:06 +00:00
|
|
|
def test_none_override_semantics(self):
|
|
|
|
|
config.e_bool = None
|
|
|
|
|
self.assertIsNone(config.e_bool)
|
|
|
|
|
for k in config._config:
|
|
|
|
|
config._config[k].user_override = _UNSET_SENTINEL
|
|
|
|
|
|
|
|
|
|
def test_reference_semantics(self):
|
|
|
|
|
config.e_list.append(2)
|
|
|
|
|
self.assertEqual(config.e_list, [1, 2])
|
|
|
|
|
config.e_set.add(2)
|
|
|
|
|
self.assertEqual(config.e_set, {1, 2})
|
|
|
|
|
config.e_dict[2] = 3
|
|
|
|
|
self.assertEqual(config.e_dict, {1: 2, 2: 3})
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
|
2024-11-01 16:26:02 +00:00
|
|
|
def test_env_name_semantics(self):
|
|
|
|
|
self.assertTrue(config.e_env_default)
|
|
|
|
|
self.assertFalse(config.e_env_default_FALSE)
|
|
|
|
|
self.assertTrue(config.e_env_force)
|
|
|
|
|
config.e_env_default = False
|
|
|
|
|
self.assertFalse(config.e_env_default)
|
|
|
|
|
config.e_env_force = False
|
|
|
|
|
self.assertTrue(config.e_env_force)
|
|
|
|
|
|
2025-01-29 23:19:05 +00:00
|
|
|
def test_env_name_string_semantics(self):
|
|
|
|
|
self.assertEqual(config.e_env_default_str, "1234")
|
|
|
|
|
self.assertEqual(config.e_env_default_str_empty, "")
|
|
|
|
|
config.e_env_default_str = "override"
|
|
|
|
|
self.assertEqual(config.e_env_default_str, "override")
|
|
|
|
|
|
2025-01-24 01:28:11 +00:00
|
|
|
def test_multi_env(self):
|
|
|
|
|
self.assertTrue(config2.e_env_default_multi)
|
|
|
|
|
self.assertTrue(config2.e_env_force_multi)
|
|
|
|
|
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
def test_save_config(self):
|
|
|
|
|
p = config.save_config()
|
2024-12-14 17:24:12 +00:00
|
|
|
self.assertDictEqual(
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
pickle.loads(p),
|
|
|
|
|
{
|
|
|
|
|
"_cache_config_ignore_prefix": ["magic_cache_config"],
|
|
|
|
|
"e_bool": True,
|
|
|
|
|
"e_dict": {1: 2},
|
|
|
|
|
"e_float": 1.0,
|
|
|
|
|
"e_int": 1,
|
|
|
|
|
"e_list": [1],
|
|
|
|
|
"e_none": None,
|
|
|
|
|
"e_set": {1},
|
|
|
|
|
"e_string": "string",
|
|
|
|
|
"e_tuple": (1,),
|
|
|
|
|
"nested.e_bool": True,
|
|
|
|
|
"_e_ignored": True,
|
|
|
|
|
"e_compile_ignored": True,
|
|
|
|
|
"magic_cache_config_ignored": True,
|
|
|
|
|
"_save_config_ignore": ["e_ignored"],
|
2024-11-01 16:13:07 +00:00
|
|
|
"e_config": True,
|
|
|
|
|
"e_jk": True,
|
|
|
|
|
"e_jk_false": False,
|
2024-11-01 16:26:02 +00:00
|
|
|
"e_env_default": True,
|
|
|
|
|
"e_env_default_FALSE": False,
|
2025-01-29 23:19:05 +00:00
|
|
|
"e_env_default_str": "1234",
|
|
|
|
|
"e_env_default_str_empty": "",
|
2024-11-01 16:26:02 +00:00
|
|
|
"e_env_force": True,
|
2024-11-07 03:49:07 +00:00
|
|
|
"e_optional": True,
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
},
|
|
|
|
|
)
|
|
|
|
|
config.e_bool = False
|
|
|
|
|
config.e_ignored = False
|
|
|
|
|
config.load_config(p)
|
|
|
|
|
self.assertTrue(config.e_bool)
|
|
|
|
|
self.assertFalse(config.e_ignored)
|
|
|
|
|
|
|
|
|
|
def test_save_config_portable(self):
|
|
|
|
|
p = config.save_config_portable()
|
2024-12-14 17:24:12 +00:00
|
|
|
self.assertDictEqual(
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
p,
|
|
|
|
|
{
|
|
|
|
|
"e_bool": True,
|
|
|
|
|
"e_dict": {1: 2},
|
|
|
|
|
"e_float": 1.0,
|
|
|
|
|
"e_int": 1,
|
|
|
|
|
"e_list": [1],
|
|
|
|
|
"e_none": None,
|
|
|
|
|
"e_set": {1},
|
|
|
|
|
"e_string": "string",
|
|
|
|
|
"e_tuple": (1,),
|
|
|
|
|
"nested.e_bool": True,
|
|
|
|
|
"e_ignored": True,
|
|
|
|
|
"e_compile_ignored": True,
|
2024-11-01 16:13:07 +00:00
|
|
|
"e_config": True,
|
|
|
|
|
"e_jk": True,
|
|
|
|
|
"e_jk_false": False,
|
2024-11-01 16:26:02 +00:00
|
|
|
"e_env_default": True,
|
|
|
|
|
"e_env_default_FALSE": False,
|
2025-01-29 23:19:05 +00:00
|
|
|
"e_env_default_str": "1234",
|
|
|
|
|
"e_env_default_str_empty": "",
|
2024-11-01 16:26:02 +00:00
|
|
|
"e_env_force": True,
|
2024-11-07 03:49:07 +00:00
|
|
|
"e_optional": True,
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
},
|
|
|
|
|
)
|
|
|
|
|
config.e_bool = False
|
|
|
|
|
config._e_ignored = False
|
|
|
|
|
config.load_config(p)
|
|
|
|
|
self.assertTrue(config.e_bool)
|
|
|
|
|
self.assertFalse(config._e_ignored)
|
|
|
|
|
|
|
|
|
|
def test_codegen_config(self):
|
|
|
|
|
config.e_bool = False
|
|
|
|
|
config.e_ignored = False
|
|
|
|
|
code = config.codegen_config()
|
|
|
|
|
self.assertEqual(
|
2024-10-29 20:38:06 +00:00
|
|
|
code,
|
|
|
|
|
"""torch.testing._internal.fake_config_module.e_bool = False
|
2025-01-07 20:04:19 +00:00
|
|
|
torch.testing._internal.fake_config_module.e_env_default = True
|
|
|
|
|
torch.testing._internal.fake_config_module.e_env_default_FALSE = False
|
2025-01-29 23:19:05 +00:00
|
|
|
torch.testing._internal.fake_config_module.e_env_default_str = '1234'
|
|
|
|
|
torch.testing._internal.fake_config_module.e_env_default_str_empty = ''
|
2025-01-30 18:39:27 +00:00
|
|
|
torch.testing._internal.fake_config_module.e_env_force = True""",
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
)
|
|
|
|
|
|
[minifier] Fix config generator for callables (#144518)
Summary:
When config contains callables, the current configs generated cannot be run:
```
torch._dynamo.config.reorderable_logging_functions = {<built-in function print>, <function warning at 0x7f774c595630>, <function log at 0x7f774c595870>, <function error at 0x7f774c595510>, <function info at 0x7f774c595750>, <built-in function warn>, <function exception at 0x7f774c5955a0>, <function debug at 0x7f774c5957e0>, <function critical at 0x7f774c5953f0>}
```
We fix the config to generate the right string, so the config is runnable, like below
```
import logging
import warnings
torch._dynamo.config.reorderable_logging_functions = { warnings.warn, logging.warn, print }
```
Test Plan:
```
buck2 run 'fbcode//mode/dev-nosan' fbcode//caffe2/test:utils -- -r test_codegen_config
```
Differential Revision: D67998703
Pull Request resolved: https://github.com/pytorch/pytorch/pull/144518
Approved by: https://github.com/desertfire
2025-01-14 17:18:13 +00:00
|
|
|
def test_codegen_config_function(self):
|
|
|
|
|
import logging
|
|
|
|
|
import warnings
|
|
|
|
|
|
|
|
|
|
config3.e_list = [print, warnings.warn, logging.warn]
|
|
|
|
|
config3.e_set = {print}
|
|
|
|
|
config3.e_func = warnings.warn
|
|
|
|
|
code = config3.codegen_config()
|
|
|
|
|
self.assertIn("import _warnings", code)
|
|
|
|
|
self.assertIn("import logging", code)
|
|
|
|
|
self.assertIn(
|
|
|
|
|
"""torch.testing._internal.fake_config_module3.e_list = ['print', '_warnings.warn', 'logging.warn']
|
|
|
|
|
torch.testing._internal.fake_config_module3.e_set = { print }
|
|
|
|
|
torch.testing._internal.fake_config_module3.e_func = _warnings.warn""",
|
|
|
|
|
code,
|
|
|
|
|
)
|
|
|
|
|
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
def test_get_hash(self):
|
2025-01-29 23:19:05 +00:00
|
|
|
hash_value = b"\x87\xf7\xc6\x1di\x7f\x96-\x85\xdc\x04\xd5\xd0\xf6\x1c\x87"
|
2024-12-14 17:24:12 +00:00
|
|
|
self.assertEqual(
|
|
|
|
|
config.get_hash(),
|
|
|
|
|
hash_value,
|
|
|
|
|
)
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
# Test cached value
|
2024-12-14 17:24:12 +00:00
|
|
|
self.assertEqual(
|
|
|
|
|
config.get_hash(),
|
|
|
|
|
hash_value,
|
|
|
|
|
)
|
|
|
|
|
self.assertEqual(
|
|
|
|
|
config.get_hash(),
|
|
|
|
|
hash_value,
|
|
|
|
|
)
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
config._hash_digest = "fake"
|
|
|
|
|
self.assertEqual(config.get_hash(), "fake")
|
|
|
|
|
|
|
|
|
|
config.e_bool = False
|
|
|
|
|
self.assertNotEqual(
|
2024-12-14 17:24:12 +00:00
|
|
|
config.get_hash(),
|
|
|
|
|
hash_value,
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
)
|
|
|
|
|
config.e_bool = True
|
|
|
|
|
|
|
|
|
|
# Test ignored values
|
|
|
|
|
config.e_compile_ignored = False
|
2024-12-14 17:24:12 +00:00
|
|
|
self.assertEqual(
|
|
|
|
|
config.get_hash(),
|
|
|
|
|
hash_value,
|
|
|
|
|
)
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
|
|
|
|
|
def test_dict_copy_semantics(self):
|
|
|
|
|
p = config.shallow_copy_dict()
|
2024-11-01 16:26:02 +00:00
|
|
|
self.assertDictEqual(
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
p,
|
|
|
|
|
{
|
|
|
|
|
"e_bool": True,
|
|
|
|
|
"e_dict": {1: 2},
|
|
|
|
|
"e_float": 1.0,
|
|
|
|
|
"e_int": 1,
|
|
|
|
|
"e_list": [1],
|
|
|
|
|
"e_none": None,
|
|
|
|
|
"e_set": {1},
|
|
|
|
|
"e_string": "string",
|
|
|
|
|
"e_tuple": (1,),
|
|
|
|
|
"nested.e_bool": True,
|
|
|
|
|
"e_ignored": True,
|
|
|
|
|
"_e_ignored": True,
|
|
|
|
|
"e_compile_ignored": True,
|
|
|
|
|
"_cache_config_ignore_prefix": ["magic_cache_config"],
|
|
|
|
|
"_save_config_ignore": ["e_ignored"],
|
|
|
|
|
"magic_cache_config_ignored": True,
|
2024-11-01 16:13:07 +00:00
|
|
|
"e_config": True,
|
|
|
|
|
"e_jk": True,
|
|
|
|
|
"e_jk_false": False,
|
2024-11-01 16:26:02 +00:00
|
|
|
"e_env_default": True,
|
|
|
|
|
"e_env_default_FALSE": False,
|
2025-01-29 23:19:05 +00:00
|
|
|
"e_env_default_str": "1234",
|
|
|
|
|
"e_env_default_str_empty": "",
|
2024-11-01 16:26:02 +00:00
|
|
|
"e_env_force": True,
|
2024-11-07 03:49:07 +00:00
|
|
|
"e_optional": True,
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
},
|
|
|
|
|
)
|
|
|
|
|
p2 = config.to_dict()
|
|
|
|
|
self.assertEqual(
|
|
|
|
|
p2,
|
|
|
|
|
{
|
|
|
|
|
"e_bool": True,
|
|
|
|
|
"e_dict": {1: 2},
|
|
|
|
|
"e_float": 1.0,
|
|
|
|
|
"e_int": 1,
|
|
|
|
|
"e_list": [1],
|
|
|
|
|
"e_none": None,
|
|
|
|
|
"e_set": {1},
|
|
|
|
|
"e_string": "string",
|
|
|
|
|
"e_tuple": (1,),
|
|
|
|
|
"nested.e_bool": True,
|
|
|
|
|
"e_ignored": True,
|
|
|
|
|
"_e_ignored": True,
|
|
|
|
|
"e_compile_ignored": True,
|
|
|
|
|
"_cache_config_ignore_prefix": ["magic_cache_config"],
|
|
|
|
|
"_save_config_ignore": ["e_ignored"],
|
|
|
|
|
"magic_cache_config_ignored": True,
|
2024-11-01 16:13:07 +00:00
|
|
|
"e_config": True,
|
|
|
|
|
"e_jk": True,
|
|
|
|
|
"e_jk_false": False,
|
2024-11-01 16:26:02 +00:00
|
|
|
"e_env_default": True,
|
|
|
|
|
"e_env_default_FALSE": False,
|
2025-01-29 23:19:05 +00:00
|
|
|
"e_env_default_str": "1234",
|
|
|
|
|
"e_env_default_str_empty": "",
|
2024-11-01 16:26:02 +00:00
|
|
|
"e_env_force": True,
|
2024-11-07 03:49:07 +00:00
|
|
|
"e_optional": True,
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
},
|
|
|
|
|
)
|
|
|
|
|
p3 = config.get_config_copy()
|
|
|
|
|
self.assertEqual(
|
|
|
|
|
p3,
|
|
|
|
|
{
|
|
|
|
|
"e_bool": True,
|
|
|
|
|
"e_dict": {1: 2},
|
|
|
|
|
"e_float": 1.0,
|
|
|
|
|
"e_int": 1,
|
|
|
|
|
"e_list": [1],
|
|
|
|
|
"e_none": None,
|
|
|
|
|
"e_set": {1},
|
|
|
|
|
"e_string": "string",
|
|
|
|
|
"e_tuple": (1,),
|
|
|
|
|
"nested.e_bool": True,
|
|
|
|
|
"e_ignored": True,
|
|
|
|
|
"_e_ignored": True,
|
|
|
|
|
"e_compile_ignored": True,
|
|
|
|
|
"_cache_config_ignore_prefix": ["magic_cache_config"],
|
|
|
|
|
"_save_config_ignore": ["e_ignored"],
|
|
|
|
|
"magic_cache_config_ignored": True,
|
2024-11-01 16:13:07 +00:00
|
|
|
"e_config": True,
|
|
|
|
|
"e_jk": True,
|
|
|
|
|
"e_jk_false": False,
|
2024-11-01 16:26:02 +00:00
|
|
|
"e_env_default": True,
|
|
|
|
|
"e_env_default_FALSE": False,
|
2025-01-29 23:19:05 +00:00
|
|
|
"e_env_default_str": "1234",
|
|
|
|
|
"e_env_default_str_empty": "",
|
2024-11-01 16:26:02 +00:00
|
|
|
"e_env_force": True,
|
2024-11-07 03:49:07 +00:00
|
|
|
"e_optional": True,
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
},
|
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
# Shallow + deep copy semantics
|
|
|
|
|
config.e_dict[2] = 3
|
|
|
|
|
self.assertEqual(p["e_dict"], {1: 2})
|
|
|
|
|
self.assertEqual(p2["e_dict"], {1: 2})
|
|
|
|
|
self.assertEqual(p3["e_dict"], {1: 2})
|
|
|
|
|
|
|
|
|
|
def test_patch(self):
|
2024-10-29 20:38:06 +00:00
|
|
|
self.assertTrue(config.e_bool)
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
with config.patch("e_bool", False):
|
|
|
|
|
self.assertFalse(config.e_bool)
|
|
|
|
|
self.assertTrue(config.e_bool)
|
|
|
|
|
with config.patch(e_bool=False):
|
|
|
|
|
self.assertFalse(config.e_bool)
|
|
|
|
|
self.assertTrue(config.e_bool)
|
|
|
|
|
with self.assertRaises(AssertionError):
|
|
|
|
|
with config.patch("does_not_exist"):
|
|
|
|
|
pass
|
|
|
|
|
|
|
|
|
|
def test_make_closur_patcher(self):
|
|
|
|
|
revert = config._make_closure_patcher(e_bool=False)()
|
|
|
|
|
self.assertFalse(config.e_bool)
|
|
|
|
|
revert()
|
|
|
|
|
self.assertTrue(config.e_bool)
|
|
|
|
|
|
2024-11-20 00:16:27 +00:00
|
|
|
def test_unittest_patch(self):
|
|
|
|
|
with patch("torch.testing._internal.fake_config_module.e_bool", False):
|
|
|
|
|
with patch("torch.testing._internal.fake_config_module.e_bool", False):
|
|
|
|
|
self.assertFalse(config.e_bool)
|
|
|
|
|
# unittest.mock has some very weird semantics around deletion of attributes when undoing patches
|
|
|
|
|
self.assertFalse(config.e_bool)
|
|
|
|
|
self.assertTrue(config.e_bool)
|
|
|
|
|
|
2024-11-20 20:59:34 +00:00
|
|
|
def test_bad_jk_type(self):
|
|
|
|
|
with self.assertRaises(
|
|
|
|
|
AssertionError,
|
|
|
|
|
msg="AssertionError: justknobs only support booleans, thisisnotvalid is not a boolean",
|
|
|
|
|
):
|
2025-02-05 19:40:10 +00:00
|
|
|
_ConfigEntry(Config(default="bad", justknob="fake_knob"))
|
2024-11-20 20:59:34 +00:00
|
|
|
|
2024-12-14 17:24:12 +00:00
|
|
|
def test_alias(self):
|
|
|
|
|
self.assertFalse(config2.e_aliasing_bool)
|
|
|
|
|
self.assertFalse(config.e_aliased_bool)
|
|
|
|
|
with config2.patch(e_aliasing_bool=True):
|
|
|
|
|
self.assertTrue(config2.e_aliasing_bool)
|
|
|
|
|
self.assertTrue(config.e_aliased_bool)
|
|
|
|
|
with config.patch(e_aliased_bool=True):
|
|
|
|
|
self.assertTrue(config2.e_aliasing_bool)
|
|
|
|
|
|
2025-01-30 18:39:27 +00:00
|
|
|
def test_reference_is_default(self):
|
|
|
|
|
t = config.e_dict
|
|
|
|
|
self.assertTrue(config._is_default("e_dict"))
|
|
|
|
|
t["a"] = "b"
|
|
|
|
|
self.assertFalse(config._is_default("e_dict"))
|
|
|
|
|
|
2025-02-05 19:40:10 +00:00
|
|
|
def test_invalid_config_int(self):
|
|
|
|
|
with self.assertRaises(AssertionError):
|
|
|
|
|
_ConfigEntry(
|
|
|
|
|
Config(default=2, env_name_default="FAKE_DISABLE", value_type=int)
|
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
def test_invalid_config_float(self):
|
|
|
|
|
with self.assertRaises(AssertionError):
|
|
|
|
|
_ConfigEntry(
|
|
|
|
|
Config(default=2, env_name_force="FAKE_DISABLE", value_type=float)
|
|
|
|
|
)
|
|
|
|
|
|
config: simplify most of the config handling and fix some bugs (#138377)
This PR combines a number of cleanups in one PR. If any of the specific cleanups don't seem to make sense, let me know and I can remove them.
Cleanups
- This PR adds a set of test suites for the config module code, which handles basically all the APIs and ways it is used. Please let me know if you see anything critical that is not tested that I missed. This test suite is primarily used as the regression test suite for later changes in this diff. Note that there is some dynamo specific testing of the config module, but it isn't as verbose.
- I removed all internal usage of shallow_copy_dict. Those usages could all use the deep copy, and did not depend on the reference behavior of certain config values that shallow_copy_dict allows.
- I removed shallow copy semantics for configuration with a deprecation warning. I think this requires a release note, so hopefully I did that correctly. Let me know if we want to continue to expose shallow copy value semantics, but I just can't find a case where I expect anyone would want it. It also complicated later internal changes to the API (i.e. breaking apart various layers of the config changes).
- I fixed what I believe is a bug in how hashes are calculated on configs. In particular, if you got the hash, then made a config change, and then got the hash again, it would not update the hash. @oulgen, please let me know if I'm misunderstanding this behavior and it is desired.
- I switched our multiple implementations of iterating through the dictionary to a single one. This is primarily to make later changes easier, but it also makes it clear how inconsistent our various config ignoring options are. Let me know if people would be interested in me unifying the various options for ignoring config values.
- I updated the test patcher (not the performance critical one, just the normal one), to use __setattr__ and __getattr__ to remove direct API access to the underlying config fetcher.
For release notes, Not sure exactly how to communicate this, but something like
"ConfigModule.to_dict, and ConfigModule.shallow_copy_dict no longer retain their shallow copy semantics, which allowed reference values objects to be modified. If you wish to modify the config object, call load_config explicitly".
Pull Request resolved: https://github.com/pytorch/pytorch/pull/138377
Approved by: https://github.com/ezyang, https://github.com/jansel, https://github.com/jovianjaison
2024-10-22 13:40:26 +00:00
|
|
|
|
|
|
|
|
if __name__ == "__main__":
|
|
|
|
|
run_tests()
|