-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[tensor] refactor param op hook #1097
Conversation
Can you add some documentation for better readability and maintainability? |
DONE |
Example:: | ||
>>> with ParamOpHookManager.use_hooks(*hooks): | ||
>>> do_something() | ||
>>> with ParamOpHookManager.use_hooks(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why nest ParamOpHookManager context?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case some users want to nest. In most cases, we don't need to nest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think users should not know what is OpHook. It is not a common-sense concept. Why do they need to have a manager?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They can extend profiler by this manager. E.g. before op sample sth, after op sample sth. Indeed, it's for advanced developers.
torch.autograd.Function
ensures the device of original inputs and grads are the same. Since zero hook may change the device of input in pre fwd, we execute pre fwd out ofautograd.Function
.