Skip to content

[Relay] improve SimplifyClipAndConsecutiveCast pass #15362

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.

Sign up for GitHub

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

Jump to bottom
Merged
Hzfengsy merged 3 commits into from
Jul 23, 2023
Merged

[Relay] improve SimplifyClipAndConsecutiveCast pass #15362

Hzfengsy merged 3 commits into from
Jul 23, 2023

Conversation

@kfeng123
Copy link
Contributor

@kfeng123 kfeng123 commented Jul 19, 2023

This PR improves SimplifyClipAndConsecutiveCast pass in several aspects.
First, it allows that the dtype of clip and the dtype of the last cast differ.
Second, it allows that there are arbitrary number of cast after clip. This is achieved via the recursion of patterns.
Both of the above two cases araise from the compilation of a prequantized model.

See the pre-RFC

@tvm-bot
Copy link
Collaborator

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

@kfeng123 kfeng123 mentioned this pull request Jul 20, 2023
Closed
@kfeng123
Copy link
Contributor Author

kfeng123 commented Jul 20, 2023

Perhaps the failure of test is due to the optimization of this PR: this PR may reduce the memory the model requires.
I file a issue.
Hopefully, some experts of usmp may help to verify this issue.

@kfeng123
Copy link
Contributor Author

kfeng123 commented Jul 20, 2023

Perhaps the failure of test is due to the optimization of this PR: this PR may reduce the memory the model requires. I file a issue. Hopefully, some experts of usmp may help to verify this issue.

For now I just modify the ground truth of the test case to pass tests. Not sure if this is correct. Correct me if the original ground truth is indeed correct.

@kfeng123 kfeng123 mentioned this pull request Jul 21, 2023
Merged
Hzfengsy
Hzfengsy reviewed Jul 21, 2023
View reviewed changes
src/relay/transforms/simplify_expr.cc
};

bool CheckDataTypeMaxMinValue(DataType dtype, double min_value, double max_value) {
bool CheckDataTypeMaxMinValue(DataType dtype, double min_value, double max_value, int mode = 0) {
Copy link
Member

@Hzfengsy Hzfengsy Jul 21, 2023

Choose a reason for hiding this comment

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

Please comment the meaning of different mode

Copy link
Contributor Author

@kfeng123 kfeng123 Jul 22, 2023

Choose a reason for hiding this comment

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

Done

@kfeng123 kfeng123 requested a review from Hzfengsy July 23, 2023 03:44
@Hzfengsy Hzfengsy merged commit 8babadb into apache:main Jul 23, 2023
@ysh329 ysh329 mentioned this pull request Oct 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Hzfengsy Hzfengsy Hzfengsy approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kfeng123 @tvm-bot @Hzfengsy