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][MSC][M0.1] Enable set name and layout for exprs #15500

Merged
merged 8 commits into from
Aug 14, 2023

Conversation

Archermmt
Copy link
Contributor

@Archermmt Archermmt commented Aug 7, 2023

This is a pull request for MSC(Multi-System Compile)
RFC: https://discuss.tvm.apache.org/t/rfc-unity-msc-introduction-to-multi-system-compiler/15251/5
Tracking issue: #15233

This is the first part of Milestone 0: Build MSCGraph core parts. Enable translation between Relay, Relax and MSCGraph without loss information.

To limit each PR in reviewable size, the Milestone 0 will be split into 5 steps:
M0.1: Passes for set name and layout for expressions (src/contrib/msc/transform)
M0.2: MSCGraph core (src/contrib/msc/core/ir/graph && python/tvm/contrib/msc/core/ir/graph)
M0.3: MSCGraph Builder (src/contrib/msc/core/ir/graph_builder)
M0.4: Codegen (src/contrib/msc/core/codegen, src/contrib/msc/framework/tvm/codegen)
M0.5: Translation test (relax/relay test && related helper modules in python)

This is the M0.1 step. Passes are added to set name and layouts via Span.
The names are essential for developing optimization algorithms, especially when developer want to control the optimization tensor by tensor. This always happens in developing of advanced quantization.
Layout is essential for pruning process, where channel relation between weights are built. And building channel relationship need layout of tensors to be set. Some quantization algorithm also need layout for quantize weights per channel or per token.

These attributes need to be set to Call, Constant as well as Var. So the best way is to find a common attributes holder for them all. In the end I chose span as the attributes holder, and special xml format like value is used to mark key:value pair in span.

cc @Hzfengsy

@tvm-bot
Copy link
Collaborator

tvm-bot commented Aug 7, 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

@github-actions github-actions bot requested a review from Hzfengsy August 7, 2023 22:07
Copy link
Member

@Hzfengsy Hzfengsy left a comment

Choose a reason for hiding this comment

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

Generally LGTM. One key problem is that how do we test MSC on CI?

python/tvm/contrib/msc/core/transform/pattern.py Outdated Show resolved Hide resolved
python/tvm/contrib/msc/core/transform/pattern.py Outdated Show resolved Hide resolved
python/tvm/contrib/msc/core/transform/pattern.py Outdated Show resolved Hide resolved
python/tvm/contrib/msc/core/transform/pattern.py Outdated Show resolved Hide resolved
@Archermmt
Copy link
Contributor Author

Archermmt commented Aug 8, 2023

Generally LGTM. One key problem is that how do we test MSC on CI?

Thanks a lot for the review!
I've just add annotations to the pattern.py.

About testing in CI:
I add some tests under tests/python/contrib/test_msc. Tests in this folder will cover most of the modules and can be used as test in CI. However, I plan to add these tests to CI after Milestone4, when end to end demos are added and most features are stable.
After the e2e demo, I will discuss with the unity on whether MSC should be used as infrastructure for model optimization and how it be further developed. If the design of MSC helps in model optimization and will be developed by other developers, CI tests are necessary. Before that, design and modules may be modified and reconstructed, tests may also be changed accordingly. So I prefer run the tests manually before PR, until the most features are stable.

@Hzfengsy
Copy link
Member

Hzfengsy commented Aug 9, 2023

Thanks @Archermmt. To be more specific, this PR does not change the CI script, and the CI will not turn on MSC. I wonder if we can turn it on (not sure if there are dependency issues). Also, please confirm that the folder tests/python/contrib/test_msc is running in the unity CI

@Archermmt
Copy link
Contributor Author

Archermmt commented Aug 9, 2023

Seems ok. Should I add the test in tests/scripts/ci.py(together with task_config_build_msc.sh && task_python_msc.sh)? I've tested with "python tests/scripts/ci.py cpu" as suggested in https://tvm.apache.org/docs/contribute/pull_request.html#guidelines

I'm not sure if that's the correct way for adding test in CI.

@Hzfengsy
Copy link
Member

Hzfengsy commented Aug 9, 2023

unity branch does not follow that way. Please add it into file tests/scripts/unity/task_python_relax.sh

@Hzfengsy
Copy link
Member

If you know how to add contrib module in CI under unity branch? @tqchen.

@yongwww
Copy link
Member

yongwww commented Aug 10, 2023

@Archermmt looks you might need to update these two files (echo set\(USE_MSC ON\) >> config.cmake in tests/scripts/task_config_build_cpu.sh and task_config_build_gpu.sh) to enable the MSC build on the CI machines for Unity

@Archermmt
Copy link
Contributor Author

Thanks, I'll try open USE_MSC in build_gpu and add the tests to scripts/unity. After Milestone2, tests on TensorRT will be added, so gpu is needed.

@Archermmt
Copy link
Contributor Author

All tests passed. Please check if there are anything else I need to change before the PR get approved. Thanks!
cc @Hzfengsy

@Hzfengsy Hzfengsy merged commit aa21782 into apache:unity Aug 14, 2023
@Hzfengsy
Copy link
Member

Thanks @Archermmt for the awesome work and reviews from @yongwww

@Archermmt Archermmt deleted the msc branch August 15, 2023 12:48
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.

4 participants