-
Notifications
You must be signed in to change notification settings - Fork 81
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
[RFC] Relay Next Roadmap #69
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @denise-k,
Thanks for publishing this, it's nice to see the start of Relax being integrated into TVM. Given it's an exploratory project in a fork, I'm wondering if this is the right kind of roadmap to have within TVM? Relax already has it's own design decisions and workflow as it operates outside of TVM and the contents of this roadmap seem to be to track the fork rather than the work to integrate it into TVM.
I'd suggest that a TVM roadmap would track the raising of RFCs for the various architectural components of Relax, and associated code integration, with a focus on integration into TVM rather than tracking the roadmap of the fork.
Thanks @Mousius for the great and insightful feedback. Let's split your feedback into two concerns which we can address separately:
Here are some actions which I believe address these concerns:
Let me know if this sounds right. |
Apologies, I'll clarify, there is no need for Relax to use any of TVMs development process, as it's an exploratory project in isolation and any pre-upstreaming part of the Relax roadmap is more useful to the Relax developers and should be maintained in the Relax repo. Therefore, I would suggest that a Relax Roadmap within TVM would instead begin part way into:
Out of scope would be
As that is when the integration work begins, this would then follow the normal TVM workflow for integrating a new feature into TVM with review within the TVM community for the RFCs and PRs. Once the initial integration work is concluded I think you're proposing for Relax work to continue within TVM, therefore the roadmapping required would be for
At which point we can figure out whether it still needs its own roadmap or whether it becomes a task on the Graph Computations and High-Level Optimizations roadmap. The Relax community would also be free to continue working in the fork and integrating into TVM as it makes sense to do so. |
Thanks folks for discussions. Just want to chime in here. I think we all agree that the development and upstreaming flow should closely follow the normal process, which is A2 as being listed by @denise-k . For emerging components being contributed to the community, it is certainly helpful for the community to have a roadmap that tracks things to be upstreamed to the codebase (that relates to contents to be upstreamed, which co-relates, but not depend on relax repo dev itself). To put it in another way, that the roadmap is independent from what/how things are being developed in relax, but more about what TVM community to expect in general as the change get checked in. The development in tvm itself still follows the normal RFC process in general (as being listed in A2). Having a separate roadmap is also important as initial integration usually starts with experimental and have a minimum impact to the current flow. From the high-level, as a community we need a way to track and maintain a global picture of things to come and provide everyone with visibility (which are aside from technical contents which are handled by the technical RFCs opened through out the upstreaming process when needed). This being said, I observe that one topic is about sequencing. e.g. should roadmap RFC be opened or merged at:
As a community that welcomes constructive improvements, it is totally fair for a roadmap RFC to be opened early before T0, and taken into action after T0. This would be a net positive for the community as there will be more informations being tracked, as long as we hold the clear standard that the items in this roadmap are served as source of truth about wants and things to come to From sequencing pov, we can wait until the first relax upstreaming RFC(T0) to merge this, but i would warmly thank @denise-k for opening the RFC and getting the community involved early. |
👍 I think we're agreed on the boundary here, Relax development activities, principles and practices aren't relevant to the
One note here, as a growing community with many closely related projects, we have to be careful of tracking a project closely until it becomes relevant to TVM itself and follows the TVM processes. To be clear, on the specific issue of Relax, there's a clear intent to contribute this back into the mainline in a near future, which indicates it is mature enough to have a roadmap and we should definitely be moving in this direction.
Given we've established the boundaries of the roadmap, do we need to wait for the first technical RFC? My concerns were only as outlined above, I don't want to hold up creating a roadmap and publicising the integration plan here as I believe that'd greatly aid the TVM community who haven't been a part of activities outside the Apache project. @denise-k I would propose we scope this roadmap to the relevant RFCs, Tracking Issues and PRs relevant to the integration of Relax into TVM, which we can immediately create and fill with placeholders to track that activity? |
update based on review comments
Thanks @Mousius, I'm aligned with the above and I've made a couple of quick updates to try and address your comments. Please note that I'm out of the office this week, so my responses may be delayed. I have asked Yuchen to shepherd this PR while I'm out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @denise-k, I'm going to merge this so you can get started on the roadmap, thank you for helping to bring the work done in Relax back into TVM 😸
cc @tqchen @areusch