Skip to content
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

Quantized _out functions don't follow same conventions as other out functions in the codebase #36508

Open
ezyang opened this issue Apr 13, 2020 · 6 comments
Assignees
Labels
better-engineering Relatively self-contained tasks for better engineering contributors low priority We're unlikely to get around to doing this in the near future oncall: quantization Quantization support in PyTorch triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@ezyang
Copy link
Contributor

ezyang commented Apr 13, 2020

Currently they're defined like:

.op("quantized::add_out(Tensor qa, Tensor qb, Tensor(a!) out)"
     "-> Tensor(a!) out",
    c10::RegisterOperators::options()
      .aliasAnalysis(at::AliasAnalysisKind::FROM_SCHEMA)
      .kernel<QAddOut</*ReLUFused=*/false>>(DispatchKey::QuantizedCPU))

However, a standard out function looks like this:

- func: angle(Tensor self) -> Tensor
  use_c10_dispatcher: full
  variants: function, method
  supports_named_tensor: True

- func: angle.out(Tensor self, *, Tensor(a!) out) -> Tensor(a!)
  supports_named_tensor: True

cc @jerryzh168 @jianyuh @raghuramank100 @jamesr66a @vkuzo @jgong5 @Xia-Weiwen @leslie-fang-intel @dzhulgakov @kevinbchen (who added the alias analysis annotations to these functions) and @z-a-f (who appears to have originally added these out variants at #23971 )

@mrshenli mrshenli added better-engineering Relatively self-contained tasks for better engineering contributors oncall: quantization Quantization support in PyTorch triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Apr 14, 2020
@supriyar
Copy link
Contributor

cc @vkuzo

@z-a-f
Copy link
Contributor

z-a-f commented Apr 15, 2020

The quantized ops live under torch.quantized.ops.XXX and are not expected to be used by the end-user in Python. However the native_functions exposes the functions in python (mostly). Do we really need to have identical apis for the two? Also, I am not sure if dispatcher schema supports *?

@dzhulgakov
Copy link
Collaborator

Only kwarg arguments should work for custom op registration api too. Switching to it is probably not BC-compatible in practice but if jit serializes explicit kwargs properly it might be fine

@ezyang
Copy link
Contributor Author

ezyang commented Apr 17, 2020

Do we really need to have identical apis for the two?

That is a great question! This isn't really related to this issue, but I don't know why quantization also registers their operators under the quantized namespace. Does anyone else know? (@jerryzh168 ?)

@andrewor14 andrewor14 added the low priority We're unlikely to get around to doing this in the near future label Nov 17, 2023
@andrewor14
Copy link
Contributor

@jerryzh168 Any context on this? Is it too late to change it now?

@jerryzh168
Copy link
Contributor

yeah this is no longer relevant I feel, since these ops are for server/edge CPU use case and used in original eager mode quantization. we have moved on in both backend (more focusing on server GPU + edge) and flow (more focusing on pt2e flow, and the current GPU + triton flow)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
better-engineering Relatively self-contained tasks for better engineering contributors low priority We're unlikely to get around to doing this in the near future oncall: quantization Quantization support in PyTorch triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

No branches or pull requests

7 participants