gromero edited a comment on pull request #8: URL: https://github.com/apache/tvm-rfcs/pull/8#issuecomment-887842848
>> 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`~ `crt_config/crt_config.h` is used and so the `MAX_` defines are used anyways? Or I missed something? -- 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: commits-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
