You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tvm.apache.org by Andrew Reusch <no...@github.com.INVALID> on 2021/06/30 20:29:21 UTC

[apache/tvm-rfcs] Add Project API RFC (#8)

This PR adds an RFC for the Project API ([embryonic discussion](https://discuss.tvm.apache.org/t/tvm-simplifying-the-compiler-interface/8703) - [m2 roadmap item #6](https://discuss.tvm.apache.org/t/tvm-microtvm-m2-roadmap/8821)`). Project API is a plugin-style infrastructure that allows TVM to integrate with a variety of platform-specific build systems. It is particularly useful to allow TVM to build and time operator implementations for non-traditional build platforms (e.g. embedded firmware) under the microTVM effort.

PR forthcoming; see PoC at https://github.com/areusch/incubator-tvm/tree/project-generator

@mehrdad @guberti @tqchen @jroesch @tkonolige @csullivan @leandron @u99127 @mousius @giuseros @gromero @stoa 
You can view, comment on, or merge this pull request online at:

  https://github.com/apache/tvm-rfcs/pull/8

-- Commit Summary --

  * Add Project API RFC

-- File Changes --

    A rfcs/0008-microtvm-project-api.md (518)

-- Patch Links --

https://github.com/apache/tvm-rfcs/pull/8.patch
https://github.com/apache/tvm-rfcs/pull/8.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm-rfcs/pull/8

Re: [apache/tvm-rfcs] Add Project API RFC (#8)

Posted by Tianqi Chen <no...@github.com.INVALID>.
@Mousius @mehrdadh @tkonolige @guberti please take another look and https://tvm.apache.org/docs/contribute/code_review.html#approve-and-request-changes-explicitly

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm-rfcs/pull/8#issuecomment-889514132

Re: [apache/tvm-rfcs] Add Project API RFC (#8)

Posted by Andrew Reusch <no...@github.com.INVALID>.
@Mousius please take another look when you can and explicitly approve!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm-rfcs/pull/8#issuecomment-887879286

Re: [apache/tvm-rfcs] Add Project API RFC (#8)

Posted by Andrew Reusch <no...@github.com.INVALID>.
Thanks @Mousius @gromero for your comments. updated the RFC to reflect accurately on PoC now that it seems to be passing regression. Please take another look so we can merge this and proceed forward with Project API.

@gromero some follow-ups on your comments:
> On generate_project method, and considering TVMC, my only comment is that it should allow a project creation based also on MLF .tar, instead of only a "live" executor.

I agree; let's merge additional logic in `project.py` to do this as follow-on to this impl.

> On build method I just think it should be a way to force a rebuild in the same project dir (i.e. even if build dir exists)

I believe that is the functionality now. Did you see something different?

> On transport, please consider my comments inline, specially the bit a about the speed regression.

Replied to your comments; I believe the speed issue is not a problem now.

> Also, if possible, please consider the comments I've posted to the draft code also (apache/tvm#8380)

Will take a look at these now that PoC is passing

> Finally, I confirm that the fixes for MAX_ defines are ok (no build error) and the per board configs are kicking in correctly.

I reworked the way these are done since posting the PoC--now `prj.conf` is generated from `microtvm_api_server.py` so we can have a single combined `template_project` and the `prj.conf` can be situation-specific. Let me know what you think.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm-rfcs/pull/8#issuecomment-886973596

Re: [apache/tvm-rfcs] Add Project API RFC (#8)

Posted by Andrew Reusch <no...@github.com.INVALID>.
@mdw-octoml would be great to get your feedback here as well

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm-rfcs/pull/8#issuecomment-871810453

Re: [apache/tvm-rfcs] Add Project API RFC (#8)

Posted by Gustavo Romero <no...@github.com.INVALID>.
>> On generate_project method, and considering TVMC, my only comment is that it should allow a project creation based also on MLF .tar, instead of only a "live" executor.
>I agree; let's merge additional logic in project.py to do this as follow-on to this impl.
Sure! I'll submit a follow up patch I've been using to test this patchset with TVMC. My comment was also more for the records for others following the discussion since we've already discussed the details about.

>> On build method I just think it should be a way to force a rebuild in the same project dir (i.e. even if build dir exists)
> I believe that is the functionality now. Did you see something different?
Yes, a functionality detail. Currently I'm handling that at TVMC side, like if the `build/` dir exists it means `tvmc micro build` was run before so it informs the user and asks to confirm a re-build using a `--force` flag. In that case, if `--force` is used TVMC will remove the `build/` and let Project API recreate it. But I was just wondering if you would like to change that behavior. I have no preference here. What do you think?

>> Also, if possible, please consider the comments I've posted to the draft code also (apache/tvm#8380)
>Will take a look at these now that PoC is passing
OK. I think you answered all my initial comments there. I'll just post a couple more related to that new round, i.e. related to last commit pushed: https://github.com/apache/tvm/commit/de75022daeb889682db80f9e90ab58b3eee898dd I have no more comments regarding the RFC itself - I'm happy with it, so I'll just post some comments about the code. I think it's pretty close to land :)

>>Finally, I confirm that the fixes for MAX_ defines are ok (no build error) and the per board configs are kicking in correctly.
> I reworked the way these are done since posting the PoC--now prj.conf is generated from microtvm_api_server.py so we can have a single combined template_project and the prj.conf can be situation-specific. Let me know what you think.

Yeah, I'm not much inclined to use that approach of using a Python code to generate the `prj.conf` because for adding new bits to the config fil will imply changing a Python code. That seems a bit strange to me. I thought of having a `prj.conf` per `project_type` but there is also other things to being handled like the stack configuration based on running the model on QEMU or a physical board, so I'm not sure, maybe we can go ahead with that approach and see it goes overtime? 

Regarding the `MAX_` defines, even with the new approach generating the `prj.conf` the `crt/crt_config.h` is used and so the `MAX_` defines are used anyways? Or I missed something?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm-rfcs/pull/8#issuecomment-887842848

Re: [apache/tvm-rfcs] Add Project API RFC (#8)

Posted by Andrew Reusch <no...@github.com.INVALID>.
@tkonolige @guberti @mehrdadh please take another look and explicitly approve if you're good w/ this

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm-rfcs/pull/8#issuecomment-887894876

Re: [apache/tvm-rfcs] Add Project API RFC (#8)

Posted by Tianqi Chen <no...@github.com.INVALID>.
Merged #8 into main.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm-rfcs/pull/8#event-5111983679

Re: [apache/tvm-rfcs] Add Project API RFC (#8)

Posted by Andrew Reusch <no...@github.com.INVALID>.
@Mousius @tqchen please take a look

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm-rfcs/pull/8#issuecomment-889484620