Re: [apache/tvm-rfcs] [Process RFC] Empowering New Scoped Module to the Project (PR #95)
@mbaret > TOSA/Linalg are both graph dialects, but they don't fulfill the same function The definition of "same" is subjective; of course, different people can have different opinions that are less grounded. For example, what if many, or even the majority of people think a proposal contains sufficient differences(e.g., in the sense of TOSA/Linalg or TorchScript/TorchFX) that also serve community needs but some don't, does that still count as "same"? The current RFC provides a guideline, considering that "An S0-module should show a clear positive fit to some(but not all) aspects of the project and clear cohesion to some of the existing modules." The "clear positive fit to some" indicates the complementary aspect(that S0-module does something other modules do not do, as a result not the same). Note such evaluation can usually be subjective. We are also not singling out MLIR as a single case here. The TorchScript/TorchFX example serves as a better example in illustrating the situations we are facing here. > The closest example I can think of is > [TCP](https://discourse.llvm.org/t/rfc-incubation-request-for-incubating-tcp-dialect-for-mlir/64883) > vs. TOSA which was prompted by this lengthy > [thread](https://discourse.llvm.org/t/rfc-proposal-for-a-high-level-ml-dialect-in-mlir/64249). > However, that caused a huge amount of debate as to whether it was correct to > have them both in the repo (similar to what we're seeing with Relax). > > The solution in the LLVM community is to have TCP first exist as an [LLVM > incubated > project](https://lists.llvm.org/pipermail/llvm-dev/2020-June/142548.html) > outside of the mono-repo. To quote the [LLVM developer > policy](https://llvm.org/docs/DeveloperPolicy.html#incubating-new-projects) > on why they have such a process: Thanks for bringing this up as a reference. Reading through the TCP thread, it is clear that the TCP proposal would not even get majority support from the MLIR core devs (most of the core devs signaled preference of not in-tree devs in that particular thread). An incubation process helps to incubate a module where the majority of the community is still unsure. We do anticipate different fork development methods will be helpful before the S0 stage. We are solving a different problem. Specifically, this process RFC outlines the guideline in situations when the majority agree that in-tree dev is helpful and the discussions of module establishment become subjective. In such cases, considering the scope of impact, users' needs and, more importantly, community empowerment in general. We think the call for the community over code and open-mindedness in the current proposal is appropriate. It also reflects G1 and aligns with the case such as TorchFX starts by offering clear positives to some aspects and continues to offer more aspects to the project. The open-mindedness of PyTorch in allowing modules(such as TorchScript and TorchFX) to co-exist contributes positively to the growth of the PyTorch ML ecosystem. As an ML project in nature and acknowledging G1 and community being critical, this proposal aims to bring that practice formally. One of the last factors is that many of us (considering many who supported the proposal) already were evaluating some of the previous RFCs following the open-mindedness principle already and pushed for community empowerment – which led to some of the RFC acceptance as well as broad inclusion of the community. If we all adopted a stronger subjective opinion and put away consideration of community, many past RFCs would have been rejected or delayed in the subjective debates, including RFCs from those who had stronger subjective opinions. We would hope to continue pushing the community over code principles. While also clarifying the clear transitions and scope of impact. -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm-rfcs/pull/95#issuecomment-1288633361 You are receiving this because you are subscribed to this thread. Message ID:
Re: [apache/tvm-rfcs] [Process RFC] Empowering New Scoped Module to the Project (PR #95)
@Hzfengsy I don't think it's fair or accurate to dismiss legitimate concerns of community contributors as 'subjective'. @areusch has already enumerated in some detail an 'objective' list of impacts that an S0 module can have on the wider project. I think at a minimum we should be addressing those points before implying that objecting to S0 modules is somehow improper (which of course it is not). What is true is that in a diverse community there are contributors who value things differently. For instance, there may be community members who put a much higher value on moving fast compared to others whose primary interest is predictability and stability. You can construct perfectly objective arguments in favour of either of these philosophies, and having both voices in the community is an enormous benefit to continuing development and success of TVM. Let's keep it that way and make sure everyone feels welcomed, recognising that constructive debate (like that happening on this very RFC) benefits us all. -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm-rfcs/pull/95#issuecomment-1288780802 You are receiving this because you are subscribed to this thread. Message ID:
Re: [apache/tvm-rfcs] [Process RFC] Empowering New Scoped Module to the Project (PR #95)
Hi, @areusch Thank you, for posting the analysis of the benefits and drawbacks of merging a module. I would like to point out that there are a few critical pieces that are missing (mainly on the community side): - Welcome new contributors who would become an added force, these contributors also contribute normally not only contributes to the particular S0 module but also to other modules. - Enable TVM to solve problems that it was not able to atm and empower new users/developers who need them. Thank you also for bringing up some of the factors to put into consideration in the RFC process: - Discussions of test strategy, modularization, and documentation during the RFC process and not land half-baked code. - Consider the relations among S0 modules to other modules, mainly on implicit impositions that S0 made on, say existing S1 APIs. - Consider the overall complexity of the codebase and presentation to the users. - Ease of maintenance overall for some of the core infra. These are great goals that we all carry when evaluating new modules(including S0) and I also support these goals. They all align with G0. The proposal text does not imply when an S0 module gets established, it will lack these considerations throughout the process. As a matter of fact, we would also take these goals into account when evaluating any proposals(including S0). One possible point of confusion here is connecting S0/S1 to importance, or how good the code is. The proposal intentionally avoids that, an earlier version of the proposal used the term "optional" which causes confusion, and we switched to a clear definition based on "scope of impact" rather than other factors such as importance, or how good the code test coverages are. An S0 module can become more important to some of our users than some of the S1 modules or have better code/tests coverage. The fact that they are more well-scoped simply means it is easier to remove them if needed, and there is less refactoring needed when refactoring other parts of the code. Admittedly, this does not mean that S0 can simply be ignored when working on other modules. It is naturally true that there are considerations around testing and API changes when working on some of the more central modules, they should be considered during RFC and follow-up actions. We want to come back and also acknowledge that the scope of impact is not binary – an S0 module in nature will have non-zero, but (sometimes significantly) less impact compared to non-S0, when it comes to changes in other modules and deprecations. This proposal calls for putting the non-binary scope of impact and community into consideration for module establishment. Of course, such consideration should be put together with other considerations, including tests, documentation, modularization, fit into the overall project, etc. All the members are expected to take these factors into account when evaluating an S0-RFC and make collective decisions for module establishment. This comes to the decision process. There are many discussions that can be made during the RFC process, which is very helpful, for example. Making sure that the proposal is technically correct, providing suggestions to testing coverage, documents, etc. All of us are taking those into account when making that collective decision. Then there comes about reasonings that are relatively subjective reasonings in module establishment. - Say we are introducing a new module to the codebase, majority of core devs think that the module provides new functionality that merits the module establishment, but some think that the new module has overlapped functionality(think about TorchFX and TorchScript case) that does not merit a module establishment. - In some cases, the majority of core devs think that the establishment of a new module to support a not yet supported by the framework outweighs the cost of extra flow maintenance while some do not. These are binary decisions that need to be made collectively by the community, they are likely subjective and different people have different pov and it is natural that people can disagree. Note that everyone also takes these factors into account. The proposal aims to provide guidelines and highlight different levels of open-mindedness. The proposal also helps to clarify the case for deprecation so we can get a cleaner codebase. Previously there wasn’t any text. Indeed we made the “S0 -> deprecation” part to be broad enough so we can handle various cases and we can talk about deprecation openly without having hard feelings about them. For example, in the case of “tests keep breaking or maintainers are relatively unresponsive”, would match this clause to start a discussion of deprecation without hard feelings. Such clarification is a positive step towards a more maintainable codebase overall. Actually having such broadness in the proposal would help, so we can welcome contribu
Re: [apache/tvm-rfcs] [Process RFC] Empowering New Scoped Module to the Project (PR #95)
@mbaret > I don't think it's fair or accurate to dismiss legitimate concerns of > community contributors as 'subjective'. @areusch has already enumerated in > some detail an 'objective' list of impacts that an S0 module can have on the > wider project. I think at a minimum we should be addressing those points > before implying that objecting to S0 modules is somehow improper (which of > course it is not). I don't want to dismiss every concern and comment in the discussion (also other feedback from the community). It spent me some time to reply to @areusch as the comments are a bit long and I'm a non-native speaker, which costs me a bit more time to organize my words. > Let's keep it that way and make sure everyone feels welcomed, recognising > that constructive debate (like that happening on this very RFC) benefits us > all. Please directly point it out if I miss any public voice. And please let me know if you feel un-welcomed during the discussion and the reasoning. I'm not a native speaker, so maybe I will make mistakes in wording and may make you uncomfortable. If so, please also let me know, and I will apologize. Again, we are working on the points and trying to address them before voting, which will absolutely benefit TVM. -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm-rfcs/pull/95#issuecomment-1288803485 You are receiving this because you are subscribed to this thread. Message ID:
