Re: [apache/tvm-rfcs] [RFC] Relax Upstreaming (PR #89)
@Mousius In this case, @YuchenJin 's reply clearly articulated that there is a close co-design of these factors, and changing to adopt dynamic alone would imply a one-step jump to relax -- which is not incremental. The data structure change would come with a set of supporting infra and co-design of things including shape inference, and other things. Of course, if we do not maintain both behavior and allow an incremental transition. Then it is equivalent to a relay=>relax bridge that would allow part of the pipeline to go through in a more incremental fashion. This approach is consistent with what is being proposed in the RFC (with clear data structure boundaries). I would suggest us not to trivialize the argument here, as it is easier to say why not do X than really go and do things. The learnings from @YuchenJin by concrete code certainly is very convincing, as well as their learnings of why not do things otherwise. This is a case where there are subjective disagreements happens. In such cases, I would encourage again for us to put community into consideration, and the fact that things are not disrupting other existing flows -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm-rfcs/pull/89#issuecomment-1266970803 You are receiving this because you are subscribed to this thread. Message ID:
Re: [apache/tvm-rfcs] [RFC] Relax Upstreaming (PR #89)
> @Mousius In this case, @YuchenJin 's reply clearly articulated that there is > a close co-design of these factors, and changing to adopt dynamic alone would > imply a one-step jump to relax -- which is not incremental. The data > structure change would come with a set of supporting infra and co-design of > things including shape inference, and other things. Co-design doesn't mean we have to land everything at once, it's often easier to prototype several features together and then break them down when we get to delivery - I believe that is most incremental and understandable rather than landing large changes with several dimensions at once. I'm also unsure how it's a one-step jump to Relax as @YuchenJin has demonstrated that the functionality of Relax is split into several different pieces. I understand that in order to land the entire proposition of Relax it may take several pieces landing, but that's natural for a large change such as this? > Of course, if we do not maintain both behavior and allow an incremental > transition. Then it is equivalent to a relay=>relax bridge that would allow > part of the pipeline to go through in a more incremental fashion. This > approach is consistent with what is being proposed in the RFC (with clear > data structure boundaries). They're not equivalent as we can avoid adding additional variations in our code by making a breaking change early and iteratively integrating the other various pieces of Relax into existing code. @slyubomirsky also expressed an interest in investigating the dynamic shapes in isolation so I believe this could have some momentum? -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm-rfcs/pull/89#issuecomment-1267200112 You are receiving this because you are subscribed to this thread. Message ID:
Re: [apache/tvm-rfcs] [RFC] Relax Upstreaming (PR #89)
Thanks for the discussion so far! Wearing Apache TVM hat, I would love to see our community making progress to satisfy broader community and work with the trend of deep learning compilation, rather than being gridlocked by a single party of interest. -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm-rfcs/pull/89#issuecomment-1267267336 You are receiving this because you are subscribed to this thread. Message ID:
Re: [apache/tvm] [Tracking Issue] Issue Triage RFC (Issue #12801)
### This issue is to track progress for [Issue Triage RFC](https://github.com/apache/tvm-rfcs/blob/main/rfcs/0093_Issue_Triage.md). This issue has these tasks: * [x] Create labels described in the RFC. * [ ] Modify ISSUE_TEMPLATES to remove Update CI Docker Image issue template and add `needs-triage` label. [[skip ci] Modify issue templates to align with Issue Tracking RFC #12898](https://github.com/apache/tvm/pull/12898) * [ ] Mark all existing issues needs-triage * [ ] Manually re-triage all issues in the codebase. * [ ] Add `triage-bot` with unit test and allow for use on labels created by a small set of beta-test users. * [ ] Expand `triage-bot` to everyone. cc @areusch -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm/issues/12801#issuecomment-1267482040 You are receiving this because you are subscribed to this thread. Message ID:
Re: [apache/tvm] [skip ci] Modify issue templates to align with Issue Tracking RFC (PR #12898)
Merged #12898 into main. -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm/pull/12898#event-7520335399 You are receiving this because you are subscribed to this thread. Message ID:
Re: [apache/tvm-rfcs] [RFC] Relax Upstreaming (PR #89)
As an individual Relax contributor from UCSD, I don’t feel our community has created an environment that welcomes new contributors and new contributions. I am happy to see different opinions in the discussion, but it's really shocking and disappointing that we are afraid of moving forward even with large amount of community support, and for such a technically strong optional module.😞 Based on several community members, Relax can solve the needs that they care about. It’s also made clear by @YuchenJin in the RFC that Relax is an optional module which does not disrupt the current TVM flow, and it does not have the intention to replace Relay. He explained in the section 6 that the three components are tightly coupled together, and explained the difficulties in simply updating Relay to bring all the benefits of Relax, but still got the same questions again and again. I am very surprised that such a module that can solve the current pain points of tvm has received so many questions about future migration plans—these are S1 and S2 stage questions, which shouldn’t be in the scope of this RFC. It's hard to imagine that everyone who wants to contribute to tvm, especially those who do foundational work like relax, needs to answer all of these questions and do future predictions about existing flow deprecation and timelines, which in my opinion discourages people from contributing. -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm-rfcs/pull/89#issuecomment-1267632697 You are receiving this because you are subscribed to this thread. Message ID:
Re: [apache/tvm-rfcs] [RFC] Relax Upstreaming (PR #89)
Thanks everyone for the feedback. One thing that we seem to agree on is that there is a strong need to support symbolic shape use cases for TVM, as represented by many of the folks who chimed into this thread. Hopefully, we all agree that there is a strong need to support robust and high-quality dynamic shape compilations, and that is not being supported in the current TVM. One of the questions is whether or not we can do that by separating the changes of C0, C1, C2 in [Rationale and Alternatives](https://github.com/YuchenJin/tvm-rfcs/blob/relax-upstream-rfc/rfcs/0089-relax-upstreaming.md#6-rationale-and-alternatives). In order to provide values to the user, we need **end-to-end** dynamic shape compilation for some specific environment of interest (for example it is less relevant at the moment for micro settings). C0, C1, and C2 are necessary to support a minimum but robust dynamic compilation: - We need to effectively lower the symbolic shape code to a sequence of calls into symbolic TensorIR functions and libraries (when we cannot generate TensorIR functions). This means in order to do so we need **C1: Add first-class support for interactions with TensorIR and TVM FFI.** - In order to further lower some of the shape handling (match_shape), we need to generate code that contains side effect (such that it allocates the buffer and writes into it). This naturally means we need **C2: Add first-class dataflow and side effect handling.** In summary, in order to build a minimum viable dynamic shape compilation in the most high quality, robust, and general fashion, the lowering flow would require stages that contain C1 and C2. We would also like to highlight that having C0, C1, and C2 helps us to achieve the goals **faster**. For example, running fusion on calls into TensorIR means we no longer need developers to annotate the fusion properties of each op, and we no longer need to handle weight layout rewrites in an ad-hoc way. This reduced engineering complexity thanks to better architecture helps us to bring robust high-quality support for dynamic shape faster to the community. The dynamic shape compilation, along with first-class C1 and C2 interaction are needs that we do not support well today in TVM. They offer clear values to our community, and differentiate from the cases that Relay already supports. One of the main topics raised was how Relax fits into TVM in general. We acknowledge that it is impossible to provide the full picture immediately. It is also not necessary to make all decisions in the beginning, as making uninformed decisions is worse. It is also common practices for OSS project to evolve and bringing in S1-conversations along the way. For example, when TorchFX got introduced, there was no prediction of TorchDynamo, which got introduced in the later part as things iterated through the codebase and became clearer. Nevertheless, we would try our best to address **how Relax fits into TVM**: C0, C1, C2 also highlight **how cohesive relax fits into TVM’s current codebase more than** many other modules in the codebase. In particular, the design deeply aligns with TensorIR, topi, symbolic shape and PackedFunc and many other modules. We also view having Relax and Relay co-exist in the codebase as a positive thing and better than the current state: - We get dynamic shape support for users who needs them, empowering the community members. - We get more developers thanks to the empowerment, offsetting the burden brought by the module. - The burden of establishing the module is minimized as it will cause no disruption to the existing modules/flows. - In many cases, having Relay and Relax work together is a positive thing: - We use Relay for importing and graph optimization and use relax for TIR-related co-optimization. It is a great combination and a better pipeline for some of the use cases. These positive parts already would bring the community a long way. We also acknowledge that we cannot pin down all the technical decisions around S1/S2 level in the very beginning. Please see the [Future Opportunities section](https://github.com/tqchen/tvm-rfcs/blob/main/rfcs/0091-establish-tvm-unity-connection.md#4-future-opportunities-after-unity-connection) in the TVM Unity connection RFC, which I encourage everyone to check out. The author also wants to come back to community empowerment. In this case, many in the community have voiced their need for empowerment with grounded technical reasonings. We would like to encourage us to take a step back, as most of the technical reasons are less grounded and we do not need to make all decisions in a single shot. We are looking at a module that is clearly isolated, makes no disruptive change to the code base, and is needed by the majority of community members. Having open-mindedness here would go a long way toward building a welcoming and inclusive community. -- Reply to this email directly or v
