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

Add pass to fold type transformations into function signature #9357

Closed
wants to merge 13 commits into from

Conversation

anwang2009
Copy link
Contributor

As title.

This transformer gives users more control over where preprocessing steps may be located, and what input types their relay program can accept

@jroesch
Copy link
Member

jroesch commented Oct 26, 2021

cc @electriclilies can you review this one?

Copy link
Contributor

@electriclilies electriclilies left a comment

Choose a reason for hiding this comment

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

Some general comments -- overall looks like you are on the right track!
(Also I realized after reviewing that this is still marked as draft, oops. let me know when it is ready for another review and I'll take a look again)

src/relay/transforms/fold_type_transformation.cc Outdated Show resolved Hide resolved
src/relay/transforms/fold_type_transformation.cc Outdated Show resolved Hide resolved
src/relay/transforms/fold_type_transformation.cc Outdated Show resolved Hide resolved
src/relay/transforms/fold_type_transformation.cc Outdated Show resolved Hide resolved
src/relay/transforms/fold_type_transformation.cc Outdated Show resolved Hide resolved
src/relay/transforms/fold_type_transformation.cc Outdated Show resolved Hide resolved
python/tvm/relay/transform/transform.py Outdated Show resolved Hide resolved
src/relay/transforms/fold_type_transformation.cc Outdated Show resolved Hide resolved
tests/python/relay/test_fold_type_transformation.py Outdated Show resolved Hide resolved
@anwang2009 anwang2009 changed the title Draft: Add pass to fold type transformations into function signature Add pass to fold type transformations into function signature Oct 26, 2021
@electriclilies
Copy link
Contributor

After taking another look, I'm a bit confused how you're doing the rewrite of the function parameters, added some questions related to that

Copy link
Contributor

@electriclilies electriclilies left a comment

Choose a reason for hiding this comment

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

Overall, LGTM, would be great if you could add one comment to the CallNode visitor saying that that logic is only for removing the casts, and also change the opt level

@masahi masahi self-assigned this Jan 9, 2022
@masahi
Copy link
Member

masahi commented Jan 9, 2022

@anwang2009 Please take a look at the CI problem.

@jroesch jroesch added the status: need update need update based on feedbacks label Jan 19, 2022
@AndrewZhaoLuo
Copy link
Contributor

@anwang2009 do you still have interest in merging this PR?

@areusch
Copy link
Contributor

areusch commented Apr 8, 2022

@AndrewZhaoLuo @anwang2009 are you guys doing this as part of #8296 or is that just related?

@AndrewZhaoLuo
Copy link
Contributor

AndrewZhaoLuo commented Apr 8, 2022

@anwang2009 any plans to continue working on this PR?

I think people do request things like this.

@anwang2009
Copy link
Contributor Author

Unrelated to that issue linked. I plan to continue working on this, I just need to find the time 🙃
Sorry for the languishing, I'll try to get something in by start of next week.

@areusch areusch added needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it and removed needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it labels Oct 19, 2022
@tqchen tqchen closed this Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: need update need update based on feedbacks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants