-
Notifications
You must be signed in to change notification settings - Fork 315
Track API usage #2706
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
base: main
Are you sure you want to change the base?
Track API usage #2706
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/2706
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit e700c81 with merge base 948ade1 ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
ccd5512
to
f6847de
Compare
@@ -101,6 +102,7 @@ def convert_to_float8_training( | |||
Returns: | |||
nn.Module: The modified module with swapped linear layers. | |||
""" | |||
torch._C._log_api_usage_once("torchao.quantization.convert_to_float8_training") |
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.
@vkuzo I put this under the torchao.quantization
namespace based on some past discussions on code reorg that hasn't happened yet. Do you prefer this namespace compared to torchao.float8
, which is the current location?
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.
also any other float8 APIs you'd like to track?
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.
is the reason for preemptively logging here ahead of the eventual migration so that querying usage metrics is easier (i.e., no need to aggregate results of 2 queries)?
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.
yeah and also we can start tracking earlier, not sure when the migration will happen
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.
IMO better to match existing code structure, and if a refactor happens we'll deal with the logging change. I say this because there is no timeline or owner for actually migrating torchao.float8
at the moment.
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.
quantize_ and pt2e looks good, these will only track internal usage right?
yeah I think so |
f6847de
to
e700c81
Compare
No description provided.