pytorch/pytorch

[dynamo] Missing support for many trivial builtin functions in operator

オープン

#116,396 opened on 2023/12/25

 (6 件のコメント) (0 件のリアクション) (0 人の担当者)Python (28,898 件のフォーク)batch import
good first issuemodule: dynamooncall: pt2triaged

Repository metrics

Stars
 (102,447 個のスター)
PR merge metrics
 (平均マージ 1d 10h) (30d で 42 merged PRs)

説明

🐛 Describe the bug

We are missing support for a few operators, operators.iconcat and operator.concat

import functools
import operator
import torch

def fn_works() -> None:
    a = list(range(1, 3))
    b = list(range(3, 6))
    c = [a, b]
    d = functools.reduce(operator.add, c, [])
    e = functools.reduce(operator.iadd, c, [])

def fn() -> None:
    a = list(range(1, 3))
    b = list(range(3, 6))
    c = [a, b]
    d = functools.reduce(operator.concat, c, [])
    e = functools.reduce(operator.iconcat, c, [])

comp_out_works = torch._dynamo.optimize(nopython=True)(fn_works)()
comp_out = torch._dynamo.optimize(nopython=True)(fn)()

Just glancing at operators documentation and the current builtin dynamo code: https://github.com/pytorch/pytorch/blob/c7e9c1510274184b41e408e6409f252bb1717085/torch/_dynamo/variables/builtin.py#L92

Things that work that I did not expect:

  • operator.is
  • operator.is_not
  • operator.__add__ # hurray for aliasing

I think we are also missing support for:

  • operator.concat
  • operator.iconcat
  • operator.contains # almost certainly broken. Be careful this when has reversed operands
  • operator.countOf
  • operator.delitem # might make sense not to fold?
  • operator.setitem # might make sense not to fold?
  • operator.abs # surprisingly broken given builtins.abs works
  • operator.length_hint # confirmed broken
  • operator.call # python 3.11+ only
  • operator.attrgetter
  • operator.itemgetter
  • operator.methodcaller
def fn_abs() -> None:
    a = range(-10, 10)
    b = map(operator.abs, a)

comp_out = torch._dynamo.optimize(nopython=True)(fn_abs)()

@XuehaiPan want to take a look at this? You did a great job fixing some of the already broken ones.

Should probably done as we can just alias it to other builtin calls.

  • operator.concat
  • operator.iconcat
  • operator.abs

Versions

Tried on latest stable, but I am sure it's on master too from looking at the source.

cc @ezyang @msaroufim @wconstab @bdhirsh @anijain2305 @zou3519 @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @aakhundov @kadeng

コントリビューターガイド