[cleanup] removing global compiler configuration #1174
Conversation
|
@vbrkicTT @chandrasekaranpradeep Also, it would be great if you added some simple sanity tests for both test sweeps and model analysis so that we can run it on push CI, increasing the likelihood that they won't be broken. |
| compiler_cfg.input_queues_on_host = input_source_flag.input_queues_on_host | ||
| if input_source_flag.set_default_dram_parameters: | ||
| compiler_cfg.default_dram_parameters = input_source_flag.default_dram_parameters | ||
| raise NotImplementedError("set_input_source is not implemented") | ||
|
|
||
| @staticmethod | ||
| def set_math_fidelity(math_fidelity: MathFidelity): | ||
| """Set compiler configuration for math fidelity""" |
There was a problem hiding this comment.
@vbrkicTT are you currently using this code path?
There was a problem hiding this comment.
Yes, in sweep tests we are currently using setting of input_queues_on_host and default_dram_parameters.
Since it's not supported in Forge, it's safe to just skip this step.
@nvukobratTT @dgolubovicTT We may rethink if we need both input sources FROM_HOST vs FROM_DRAM_QUEUE. They use the same model it's just this config which is different.
There was a problem hiding this comment.
Let's put this as a topic to confirm. Right now we're in process of changing where and how our inputs are stored, and planing how we're going to handle them in the future. Once we come to reasonable conclusions, we should update this part as well.
There was a problem hiding this comment.
Ok, for new we can keep behavior of sweep tests the same.
@pilkicTT @nvukobratTT Adaptation to the change for sweep tests is prepared in #1195, and can go sometime after this PR as a follow-up
|
||||||||||||||
1 similar comment
|
||||||||||||||
|
||||||||||||||
1 similar comment
|
||||||||||||||
Sure I will create a sanity test for model analysis and add it in push pipeline |
The PR removes the dependency of model analysis pipeline on global compiler config. The `compiler_cfg.tvm_generate_unique_ops_tests` and `compiler_cfg.extract_tvm_unique_ops_config` compiler config are used for extracting the unique ops configuration from TVM and generate unique ops test for extracted configuration. The `compiler_cfg.tvm_generate_unique_ops_tests` and `compiler_cfg.extract_tvm_unique_ops_config` compiler config are enabled through `initialize_global_compiler_configuration_based_on_pytest_args` fixture in `forge/test/conftest.py` file by specifying the `--generate-unique-ops-tests` or `--extract-tvm-unique-ops-config` pytest options while running the models test Since [this PR](#1174) remove the global compiler configuration, removed the dependency of enabling `compiler_cfg.tvm_generate_unique_ops_tests` and `compiler_cfg.extract_tvm_unique_ops_config` compiler config through `--generate-unique-ops-tests` or `--extract-tvm-unique-ops-config` pytest options and now the `compiler_cfg.tvm_generate_unique_ops_tests` and `compiler_cfg.extract_tvm_unique_ops_config` compiler config are enabled through **FORGE_TVM_GENERATE_UNIQUE_OPS_TESTS** or **FORGE_EXTRACT_TVM_UNIQUE_OPS_CONFIG** environment variable
pilkicTT
force-pushed
the
pilkic/compiler-cfg
branch
from
7d2a7d7 to
c2cda48
Compare
|
||||||||||||||
1 similar comment
|
||||||||||||||
|
||||||||||||||
1 similar comment
|
||||||||||||||
pilkicTT
force-pushed
the
pilkic/compiler-cfg
branch
from
c2cda48 to
637149d
Compare
|
||||||||||||||
|
||||||||||||||
|
||||||||||||||
|
||||||||||||||
| compiler_cfg.input_queues_on_host = input_source_flag.input_queues_on_host | ||
| if input_source_flag.set_default_dram_parameters: | ||
| compiler_cfg.default_dram_parameters = input_source_flag.default_dram_parameters | ||
| raise NotImplementedError("set_input_source is not implemented") | ||
|
|
||
| @staticmethod | ||
| def set_math_fidelity(math_fidelity: MathFidelity): | ||
| """Set compiler configuration for math fidelity""" |
There was a problem hiding this comment.
Let's put this as a topic to confirm. Right now we're in process of changing where and how our inputs are stored, and planing how we're going to handle them in the future. Once we come to reasonable conclusions, we should update this part as well.
This is a part of ongoing effort to remove any global state left over from pybuda. There should be no need of having global configuration of compiler. If user wants to override some of the exposed settings, it should be done locally, by creating a new (default) config and then passing it into the compile function. All tests which need to modify the config should do the same. No feature should rely on reading from a "volatile global configuration". Instead, each feature must either be designed to receive the user-specified compiler configuration directly or defer config-dependent decisions for later (until the compilation stage where the appropriate configuration is available). In majority of the cases I would assume that direct access to the user passed config is easily available. Closes #147
pilkicTT
force-pushed
the
pilkic/compiler-cfg
branch
from
637149d to
3dd4321
Compare
|
||||||||||||||
1 similar comment
|
||||||||||||||
|
||||||||||||||
|
||||||||||||||
### Ticket Fix for #147 ### Problem description Global compiler configuration completely removed in #1174. Parameters input_queues_on_host and default_dram_parameters removed. Parameter default_math_fidelity still exists, though still not respected. ### What's changed Maintain (instantiate and pass) CompilerConfig through sweep test verification. ### Checklist - [ ] New/Existing tests provide coverage for changes
This is a part of ongoing effort to remove any global state left over from pybuda.
There should be no need of having global configuration of compiler. If user wants to override some of the exposed settings, it should be done locally, by creating a new (default) config and then passing it into the compile function. All tests which need to modify the config should do the same.
No feature should rely on reading from a "volatile global configuration". Instead, each feature must either be designed to receive the user-specified compiler configuration directly or defer config-dependent decisions for later (until the compilation stage where the appropriate configuration is available).
In majority of the cases i would assume that direct access to the user passed config is easily available.
Closes #147