Skip to content

[microTVM] Arduino: Fix MLF archive filename in generated project dir#9320

Merged
leandron merged 2 commits into
apache:mainfrom
gromero:microtvm-fix-arduino-mlf-filename
Oct 27, 2021
Merged

[microTVM] Arduino: Fix MLF archive filename in generated project dir#9320
leandron merged 2 commits into
apache:mainfrom
gromero:microtvm-fix-arduino-mlf-filename

Conversation

@gromero

@gromero gromero commented Oct 19, 2021

Copy link
Copy Markdown
Contributor

Currently generate_project API method is copying the input MLF archive
filename without renaming it to "model.tar" - hence not in accordance
with the specification. As a consequence when the server looks for that
file to determine if it's a project dir or a template dir it always
determines it is a template dir since "model.tar" can never be found, so
a TemplateProjectError() exception is thrown when instantiating a
GeneratedProject class.

This commit fixes that by correctly copying the input MLF archive to
the newly generated project dir as "model.tar" so the server can find
it.

It also takes the chance to change the MLF path returned by
server_info_query method: only if it's not a template dir the MLF path
is returned, otherwise an empty string is returned (it doesn't make
sense to return a MLF path when it's a template dir because there isn't
any model associated to a template dir).

Signed-off-by: Gustavo Romero gustavo.romero@linaro.org

Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

Currently generate_project API method is copying the input MLF archive
filename without renaming it to "model.tar" - hence not in accordance
with the specification. As a consequence when the server looks for that
file to determine if it's a project dir or a template dir it always
determines it is a template dir since "model.tar" can never be found, so
a TemplateProjectError() exception is thrown when instantiating a
GeneratedProject class.

This commit fixes that by correctly copying the input MLF archive to
the newly generated project dir as "model.tar" so the server can find
it.

It also takes the chance to change the MLF path returned by
server_info_query method: only if it's not a template dir the MLF path
is returned, otherwise an empty string is returned (it doesn't make
sense to return a MLF path when it's a template dir because there isn't
any model associated to a template dir).

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
@gromero

gromero commented Oct 19, 2021

Copy link
Copy Markdown
Contributor Author

cc @mehrdadh @guberti @areusch

@gromero

gromero commented Oct 19, 2021

Copy link
Copy Markdown
Contributor Author

Found when reviewing #9289

@Mousius

Mousius commented Oct 19, 2021

Copy link
Copy Markdown
Member

@gromero is it possible to replicate the scenario in a test?

@gromero

gromero commented Oct 19, 2021

Copy link
Copy Markdown
Contributor Author

@gromero is it possible to replicate the scenario in a test?

@Mousius Yes, below is a repro. So all a test needs to do is generate a project dir based on Arduino default template. I haven't looked closely why current tests don't catch it (I tried to run them for Arduino "due" board and they pass, afaics they are not flashing anything to the board as we do on Zephyr tests...).

>>> import tvm.micro.project as p
>>> prj  = p.generate_project_from_mlf("/home/gromero/git/tvm/apps/microtvm/arduino/template_project","/tmp/4300","/home/gromero/git/tvm/python/tvm/driver/tvmc/sine.tar", { "project_type": "host_driven", "arduino_board": "due", "arduino_cli_cmd": "/home/gromero/bin/arduino-cli" } )
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/gromero/git/tvm/python/tvm/micro/project.py", line 199, in generate_project_from_mlf
    return template.generate_project_from_mlf(str(mlf_path), str(project_dir), options)
  File "/home/gromero/git/tvm/python/tvm/micro/project.py", line 122, in generate_project_from_mlf
    return GeneratedProject.from_directory(project_dir, options)
  File "/home/gromero/git/tvm/python/tvm/micro/project.py", line 68, in from_directory
    return cls(client.instantiate_from_dir(project_dir), options)
  File "/home/gromero/git/tvm/python/tvm/micro/project.py", line 75, in __init__
    raise TemplateProjectError()
tvm.micro.project.TemplateProjectError
>>> 

@guberti guberti left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but we should add a test. LMK if you want me to take a crack at writing the test!

@gromero

gromero commented Oct 22, 2021

Copy link
Copy Markdown
Contributor Author

@guberti Thanks for the review. Yes, please, if you can crack take a crack at writing that it would be cool.

@leandron leandron left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - I assume @guberti will send a test case in a separate PR, so we can merge this for now to fix the issue.

@leandron leandron merged commit 03665a3 into apache:main Oct 27, 2021
@gromero

gromero commented Oct 27, 2021

Copy link
Copy Markdown
Contributor Author

LGTM - I assume @guberti will send a test case in a separate PR, so we can merge this for now to fix the issue.

@leandron Yeah that's right, it will be a follow-on PR. Thanks for the review 👍

ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
…apache#9320)

* [microTVM] Arduino: Fix MLF archive filename in generated project dir

Currently generate_project API method is copying the input MLF archive
filename without renaming it to "model.tar" - hence not in accordance
with the specification. As a consequence when the server looks for that
file to determine if it's a project dir or a template dir it always
determines it is a template dir since "model.tar" can never be found, so
a TemplateProjectError() exception is thrown when instantiating a
GeneratedProject class.

This commit fixes that by correctly copying the input MLF archive to
the newly generated project dir as "model.tar" so the server can find
it.

It also takes the chance to change the MLF path returned by
server_info_query method: only if it's not a template dir the MLF path
is returned, otherwise an empty string is returned (it doesn't make
sense to return a MLF path when it's a template dir because there isn't
any model associated to a template dir).

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>

* Retrigger CI
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
…apache#9320)

* [microTVM] Arduino: Fix MLF archive filename in generated project dir

Currently generate_project API method is copying the input MLF archive
filename without renaming it to "model.tar" - hence not in accordance
with the specification. As a consequence when the server looks for that
file to determine if it's a project dir or a template dir it always
determines it is a template dir since "model.tar" can never be found, so
a TemplateProjectError() exception is thrown when instantiating a
GeneratedProject class.

This commit fixes that by correctly copying the input MLF archive to
the newly generated project dir as "model.tar" so the server can find
it.

It also takes the chance to change the MLF path returned by
server_info_query method: only if it's not a template dir the MLF path
is returned, otherwise an empty string is returned (it doesn't make
sense to return a MLF path when it's a template dir because there isn't
any model associated to a template dir).

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>

* Retrigger CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants