Dynamo+LTC: add pybind to set force fallback config and use that in test_extract_compiled_graph.py #75292
Conversation
…est_extract_compiled_graph.py [ghstack-poisoned]
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit a1d444d (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
…e that in test_extract_compiled_graph.py" - Follow the convention in [this doc](https://docs.google.com/document/d/1Vi96ITGoK7BW01ZEccexs4pvCQKF4_LdV8w7TfIWPvM/edit) to setup config for ltc force fallback ops. - Pybinds are added to read/set the config. - Use the added pybinds in the unit test which needs to force fallbacks. Test plan: ``` pytest test/lazy/test_extract_compiled_graph.py ``` [ghstack-poisoned]
| try: | ||
| yield None | ||
| finally: | ||
| config.set_force_fallback("") |
There was a problem hiding this comment.
maybe better to first do
_orig_force_fallback = config.get_force_fallback()
finally
config.set_force_fallback(_orig_force_fallback)
?
probably unlikely to use multiple values though..
There was a problem hiding this comment.
yep, that's better.
|
@shunting314 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
| namespace torch { | ||
| namespace lazy { | ||
|
|
||
| std::string& getLTCForceFallback() { |
There was a problem hiding this comment.
i'm thinking of changing the name to fit in with the torch_lazy pattern, but i haven't really thought of what the right name scheme is. i think we should land this first and then we can consider names later. my suggestion might be torch::lazy::get_force_fallback but i wonder what other people think. Should 'config' be a namespace, or any other proposal?
There was a problem hiding this comment.
Both getLTCForceFallback/getForceFallback and get_force_fallback sounds good as long as we are consistent about the flavor.
wconstab
left a comment
wconstab
left a comment
There was a problem hiding this comment.
awesome thanks for following up on this @shunting314 💯
…e that in test_extract_compiled_graph.py" - Follow the convention in [this doc](https://docs.google.com/document/d/1Vi96ITGoK7BW01ZEccexs4pvCQKF4_LdV8w7TfIWPvM/edit) to setup config for ltc force fallback ops. - Pybinds are added to read/set the config. - Use the added pybinds in the unit test which needs to force fallbacks. Test plan: ``` pytest test/lazy/test_extract_compiled_graph.py ``` Differential Revision: [D35417678](https://our.internmc.facebook.com/intern/diff/D35417678) [ghstack-poisoned]
|
@shunting314 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
…est_extract_compiled_graph.py (#75292) Summary: Pull Request resolved: #75292 - Follow the convention in [this doc](https://docs.google.com/document/d/1Vi96ITGoK7BW01ZEccexs4pvCQKF4_LdV8w7TfIWPvM/edit) to setup config for ltc force fallback ops. - Pybinds are added to read/set the config. - Use the added pybinds in the unit test which needs to force fallbacks. Test Plan: ``` pytest test/lazy/test_extract_compiled_graph.py ``` Reviewed By: malfet Differential Revision: D35417678 Pulled By: shunting314 fbshipit-source-id: 1e05b8c831174872d70257a0ddd958863d6ca80d
|
Hey @shunting314. |
|
Hello, I am wondering whether there is a way for me to fallback all ops with a flag? |
| static char* force_str = std::getenv("LTC_FORCE_FALLBACK"); | ||
| if (force_str != nullptr) { | ||
| auto force_str = getLTCForceFallback(); | ||
| if (!force_str.empty()) { |
There was a problem hiding this comment.
@yxd886 this is the code that actually implements the force fallback.
you could modify it to accept a list of ops separated by a comma or something, but currently it only supports a single op, and its both more complex and less efficient to set it up for a long list of many ops.
if you want another mode that just falls back all the time, you could add an env for that and make this function just always return true based on that env.
Stack from ghstack (oldest at bottom):
Test plan:
Differential Revision: D35417678