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

[Unity][FIX] add init file to relax.backend.contrib #15023

Merged
merged 2 commits into from
Jun 5, 2023

Conversation

Hzfengsy
Copy link
Member

@Hzfengsy Hzfengsy commented Jun 5, 2023

This PR adds __init__.py to relax.backend.contrib, fixing the package issue reported at mlc-ai/mlc-llm#311

cc @yzh119 @tqchen

This PR adds `__init__.py` to `relax.backend.contrib`, fixing the package
issue reported at mlc-ai/mlc-llm#311
@tvm-bot
Copy link
Collaborator

tvm-bot commented Jun 5, 2023

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.

Generated by tvm-bot

@Hzfengsy Hzfengsy changed the title [FIX] add init file to relax.backend.contrib [Unity][FIX] add init file to relax.backend.contrib Jun 5, 2023
@github-actions github-actions bot requested review from tqchen and yzh119 June 5, 2023 11:27
@masahi
Copy link
Member

masahi commented Jun 5, 2023

The file was removed recently to fix a circular dep problem #15001

@Hzfengsy
Copy link
Member Author

Hzfengsy commented Jun 5, 2023

@masahi Thanks for sharing. However, __init__ file is needed as setuptools will ignore files if there is no __init__ file in this folder. Maybe we need to consider an alternate way to avoid circular dep.

# under the License.
"""Relax backends contrib"""
from . import cublas
from . import cutlass
Copy link
Member

Choose a reason for hiding this comment

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

Does keeping an empty init file fix the issue? We don't want from . import cutlass (probably cublas as well) here.

Copy link
Member

@masahi masahi Jun 5, 2023

Choose a reason for hiding this comment

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

or maybe it is fine to import cutlass as long as we are not importing partition_for_cutlass like previously.

If

from tvm.contrib.cutlass import (
    has_cutlass,
    num_cutlass_partitions,
    finalize_modules
)

doesn't get an error, I'm cool with this.

Copy link
Member Author

Choose a reason for hiding this comment

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

The empty init file is fine.

Copy link
Member

@masahi masahi Jun 5, 2023

Choose a reason for hiding this comment

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

Here is the summary of the previous issue for reference:

  • It is fine (for now) for backend/contrib/cutlass.py to depend on contrib/cutlass/build.py
  • But contrib/cutlass/build.py does import relax, which transitively imports backend/contrib/cutlass.py

@masahi masahi merged commit 1a4e08e into apache:unity Jun 5, 2023
yzh119 pushed a commit to mlc-ai/relax that referenced this pull request Jun 5, 2023
@Hzfengsy Hzfengsy deleted the add_init_file branch June 6, 2023 06:03
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.

3 participants