-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Add register_op_hook for gluon #15839
Add register_op_hook for gluon #15839
Conversation
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.
Very nice!
f4a24cd
to
1adbffa
Compare
1adbffa
to
75a1321
Compare
It looks like my changed files are not being picked up and probably cached files are being picked up for CI. @marcoabreu would you know how I can force clear the cache ? |
…to monitor_callback_gluon
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.
Nice!! Looks good !
Have we measured how much performance overhead this might introduce? |
@apeforest no we haven't measured perf overhead since This is only used for monitoring. This should not be used in production or for perf intensive use cases. |
Thank you @Vikas-kum, @rahul003, @apeforest for the review!! |
* Set monitor callback basic support * Trigger CI * Add base.pyi and ndarray.pyx * Change not supported to experimental and check for both static_shape and static_alloc
@anirudh2290 It seems this pr may affect the trace results, below shows the visualized results using chrome://tracing when training a bert model. |
@joapolarbear do you just see much spacing out or is it that ops are missing in one but not in other. registering an op hook is expensive and called at the beginning and end of each op execution and may kill performance. It is meant to be used for debugging and should be commented out during profiling. |
@anirudh2290 did we measure how much overhead it is to invoke a dummy hook? |
@eric-haibin-lin i didnt measure that, since this was intended only for debugging not to be used in production use cases. If I had to make a guesstimate i would say it would be very similar to invoking a dummy hook with set_monitor_callback in module. |
assert self._cached_op, "cached op is not None" | ||
if self._callback: | ||
self._cached_op._register_op_hook(self._callback, self._monitor_all) | ||
if len(self._flags) >= 2 and (self._flags[1] or self._flags[0]): |
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.
self._flags = list(kwargs.items())
The condition is true even when it is not supposed to be. For example if
self._flags = [('static_alloc', False), ('static_shape', False), ('inline_limit', 2)]
Description
This PR adds a register_op_hook for Block. One can register_op_hook to monitor all the ops that are called by the HybridBlock after hybridization. For example:
This will print the following :
Setting monitor_all=False will print only the output :
What is not supported
cc @Vikas-kum
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments