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

[Relay] fix: add compute tag for trilu #13120

Merged
merged 1 commit into from
Oct 18, 2022
Merged

[Relay] fix: add compute tag for trilu #13120

merged 1 commit into from
Oct 18, 2022

Conversation

ganler
Copy link
Contributor

@ganler ganler commented Oct 18, 2022

@tvm-bot
Copy link
Collaborator

tvm-bot commented Oct 18, 2022

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

  • No users to tag found in teams: relay See #10317 for details
  • Built docs for commit e728d52 can be found here.

Generated by tvm-bot

@ganler ganler changed the title fix: add compute tag for trilu [Relay] fix: add compute tag for trilu Oct 18, 2022
Copy link
Contributor

@shingjan shingjan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the test is passing then I guess it is fine. Just wonder if we need to add trilu here as an injective op.

@ganler
Copy link
Contributor Author

ganler commented Oct 18, 2022

@shingjan Thanks.

Adding that seems to bring a new error:

"""
  File "/home/jiawei/dev/tvm-pr/python/tvm/relay/op/__init__.py", line 54, in <module>
    from . import _transform
  File "/home/jiawei/dev/tvm-pr/python/tvm/relay/op/_transform.py", line 56, in <module>
    _reg.register_injective_schedule("trilu")
  File "/home/jiawei/dev/tvm-pr/python/tvm/relay/op/op.py", line 279, in register_injective_schedule
    return register_schedule(op_name, _schedule_injective, level)
  File "/home/jiawei/dev/tvm-pr/python/tvm/relay/op/op.py", line 264, in register_schedule
    fstrategy = _create_fstrategy_from_schedule(op_name, schedule)
  File "/home/jiawei/dev/tvm-pr/python/tvm/relay/op/op.py", line 195, in _create_fstrategy_from_schedule
    assert compute is not None, "FTVMCompute is not registered for op %s" % op_name
AssertionError: FTVMCompute is not registered for op trilu
"""

Copy link
Contributor

@shingjan shingjan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like adding this as an injective schedule will requires the FTVMCompute to be added, which may not be what we want as the topi trilu is implemented in python. Looks good to me with the original change!

@masahi masahi merged commit 3d22dbf into apache:main Oct 18, 2022
@ganler ganler deleted the trilu-tag branch October 18, 2022 19:33
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 10, 2022
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] trilu not tagged with injective and thus miss reduce schedule
4 participants