tqchen commented on PR #13926:
URL: https://github.com/apache/tvm/pull/13926#issuecomment-1420987354
First of all, it is important indeed important to group things into
reasonably isolated components that have reasonable structure.
That does not mean, however, that such logical structures do not require a
broader context to be able to understand and iterate. For example, knowing IR
nodes themselves is indeed a valuable point of assessment. But to fully access
it, we will need to look at the context of how an IR node interacts with
parser, printer, and passes. These are contexts that would be helpful to obtain
over an extended course of time.
Coming back to the state of branch establishment. There are generally two
different ways of establishing the module.
- W0: Start with an initial rough subset of implementations, then iterate to
expand the feature
- As time goes by, these modules have features that get expanded into a
broader set. The scaffolding commits are still going to be large. Then small
commits are built on top.
- W1: Have a reasonable subset component that gets planned with a decent set
of features, documents, and tests. Such scaffolding PRs can indeed contain more
lines of code (and requires levels of expertise of the overall architecture)
mainly because that represents a squashed view of part of history in W0.
As someone who has ramped up many OSS projects and modules, I would like to
say that such a period is not a normal steady state but sometimes can be
necessary for the ramping period both in W0 and W1 for us to flexibly bring in
pieces. Right now, given our current understanding of unity, we are in a
W1-style branch establishment stage.
Coming back to the technical assessment of this PR. Objectively, I think it
is sufficiently isolated in terms of a logical component, `BlockBuilder` which
comes with struct info deduction for the full IR. And related minimum operator
and te-emission support so to enable a good amount of test coverage. It is
indeed long in terms of lines of code, but from an architectural pov clear in
terms of how things are isolated.
If there is an actionable suggestion on further isolating things, this could
be great feedback, and we could say rewrite the history of the branch to
introduce such a split. The same goes for, say, a suggestion on commit
messages. These follow-up comments do have a lot of meanings and are valuable.
As again, we can view them as review suggestions to the branch.
Similarly, with some additional context through an extended period of time,
we help community members gain a deeper understanding to give more concrete,
actionable suggestions. These feedbacks are still valuable and sometimes even
more informative throughout development.
The flexibility of branch development offers us a way to react to those
comments, and we welcome them wholeheartedly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
