[ca] no longer require is_traceable annotations for c++ autograd functions #146229
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/146229
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ✅ You can merge normally! (2 Unrelated Failures)As of commit b313e9f with merge base 87a63a9 ( UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
| auto fn_name = pyinterface->bind_function( | ||
| saved.get_py_compiler(), | ||
| std::string(typeid(T).name()), | ||
| CppNode_apply_functional_ivalue<T>, | ||
| schema, | ||
| /*is_custom_function*/ true); | ||
| /*is_custom_function*/ true, | ||
| /*is_traceable*/ T::is_traceable); |
There was a problem hiding this comment.
What's the default for is_traceable?
There was a problem hiding this comment.
false, and always will have to be false
torch/_dynamo/compiled_autograd.py
Outdated
| if is_traceable: | ||
| torch._dynamo.allow_in_graph(result) |
There was a problem hiding this comment.
Otherwise, what causes Dynamo to graph break on it? the SkipFiles error?
(It might be better for readability and for the error messages+logging if we explicitly torch._dynamo.skip the non-traceable things)
There was a problem hiding this comment.
something about pybind. changed it to a disable, and had to wrap it in a fn to make the decorator work
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
This PR accumulates comple reasons inside each CacheNode, and logs them to tlparse on each CA compile. This defines a compile as an autograd structure change, and a recompile as a dynamic shape change. sample tlparse: https://manifold.edge.x2p.facebook.net/v0/read/tree/logs/.tmpdbo7gt/index.html?bucketName=tlparse_reports&apiKey=tlparse_reports-key&withPayload=1&timeoutMsec=100 for compiles: ```python [ "!0: Cache miss due to new autograd node: torch::autograd::GraphRoot (NodeCall 0) with key size 39, previous key sizes=[]" ] ``` for recompiles: ```python [ "!0: Cache miss due to new autograd node: torch::autograd::GraphRoot (NodeCall 0) with key size 39, previous key sizes=[]", "!1: Cache miss due to 7 changed tensor shapes (total of 7): sizes[0], sizes[1], sizes[2], sizes[3], sizes[4], sizes[5], sizes[6]" ] ``` Pull Request resolved: #146386 Approved by: https://github.com/jansel ghstack dependencies: #146229
Stack from ghstack (oldest at bottom):
This PR removes the CA compile-time error for C++ autograd functions, and supports them by having dynamo graph break on them (instead of allow_in_graph). The CppNode's collects are kept as is for now.
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @amjames @desertfire @chauhang @aakhundov