Introduce unsafe way to mark functions as cacheable #151603
Conversation
[ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/151603
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: ❌ 1 Cancelled Job, 12 Unrelated FailuresAs of commit 83e2d32 with merge base c3a7278 ( CANCELLED JOB - The following job was cancelled. Please retry:
FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov [ghstack-poisoned]
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov [ghstack-poisoned]
| "unsafe_marked_cacheable_functions", | ||
| ["torch.ops.higher_order.tag_activation_checkpoint"], |
There was a problem hiding this comment.
does this work with non-HOPs?
The way to test this is to create a new function, then use torch._dynamo.allow_in_graph on it (so it shows up in the Dynamo graph), and then try to see if it can be cached
| return ( | ||
| self._cacheable | ||
| or f"{self.__module__}.{self.__name__}" | ||
| in torch._inductor.config.unsafe_marked_cacheable_functions |
There was a problem hiding this comment.
Just a thought - I'm a bit worried about a situation like:
(1) end user is running torch.compile
(2) their compiled code uses some 3rd-party library code that (incorrectly) marks a bad function as cache-safe
(3) they get silent correctness and report it to us.
One obvious debugging step would be to ask them to find any of these functions that have been marked cache-safe and turn off caching for them, so we can easily tell if it's our fault (general caching bug) or someone else's fault (3rd party lib doing unsafe things).
This will be a pain to do in the current setup - since even if we tell the user to set unsafe_marked_cacheable_functions=[], the user doesn't have an easy way of ensuring that they can update this config last, after any 3rd party libs add to the config.
Given this setup - what do you think of adding (yet another) config to "ignore" these markings?
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov [ghstack-poisoned]
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov [ghstack-poisoned]
|
@pytorchbot merge -f "ci says it is safe to land" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Stack from ghstack (oldest at bottom):
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @chenyang78 @kadeng @muchulee8 @amjames @chauhang @aakhundov