-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Add api usage log to transforms #5007
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
Conversation
💊 CI failures summary and remediationsAs of commit 5d46f89 (more details on the Dr. CI page):
1 failure not recognized by patterns:
1 job timed out:
This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
Currently we can't log IMO we could skip |
@kazhang @fmassa The policy on whether the call should be added on a constructor VS functional part changes from area to area. On the operators, I believe Francisco opted for adding the call only once, carefully adding it mainly on functional and secondary on constructors (only for those Ops with no functional component). On the other hand Kai proposes to add it to all constructor and functions but excludes those that are too simple (like |
Agree with this simple approach, log API is only called once and pretty lightweight. We could further dedup in the downstream if needed. |
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.
Thanks @kazhang, overall looks good. Just one comment, let me know what you think.
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.
LGTM, thanks @kazhang
Hey @datumbox! You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py |
Summary: * add api usage log to functional transforms * add log to transforms * fix for scriptablity * skip Compose for scriptability * follow the new policy * torchscriptbility * adopt new API * make Compose scriptable * move from __call__ to __init__ Reviewed By: prabhat00155 Differential Revision: D33253462 fbshipit-source-id: c9e27e573869c84f2c0df295ec9f5542a327126f
RandomCrop
), even though crop is logged in functional transforms, so that we could distinguish whether thecrop
is called from a random transform or used directly;ToTensor
partially-close: API usage logging within TorchVision #5052
partially-close: Add
_log_api_usage_once()
calls on remaining endpoints #4955cc @vfdev-5 @datumbox