[GitHub] [tvm] gromero commented on a change in pull request #9026: [microTVM][Zephyr] Add 'main_stack_size' option to API server

[GitBox <gi...@apache.org>]

gromero commented on a change in pull request #9026:
URL: https://github.com/apache/tvm/pull/9026#discussion_r712203347



##########
File path: tests/micro/zephyr/test_zephyr.py
##########
@@ -416,23 +407,21 @@ def test_autotune_conv2d(temp_dir, board, west_cmd, tvm_debug):
         tasks = tvm.autotvm.task.extract_from_program(mod["main"], {}, target)
     assert len(tasks) > 0
 
-    repo_root = pathlib.Path(
-        subprocess.check_output(["git", "rev-parse", "--show-toplevel"], encoding="utf-8").strip()
-    )
+    if test_utils.qemu_boards(board):

Review comment:
       Same thing here, you need to set `config_main_stack_size` in case it's not a QEMU board, otherwise the test will fail.

##########
File path: tests/micro/zephyr/test_zephyr.py
##########
@@ -58,21 +49,23 @@ def _make_sess_from_op(
 
 
 def _make_session(temp_dir, zephyr_board, west_cmd, mod, build_config):
-    stack_size = None
-    if conftest.qemu_boards(zephyr_board):
-        stack_size = 1536
+    if test_utils.qemu_boards(zephyr_board):
+        config_main_stack_size = 1536

Review comment:
       In this case you do need to set `config_main_stack_size = None` because if the board is not a qemu one the code will crash because `config_main_stack_size` will never be set. 

##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -260,12 +261,7 @@ def _get_nrf_device_args(options):
         help="Name of the Zephyr board to build for.",
     ),
     server.ProjectOption(
-        "zephyr_model",
-        choices=[board["model"] for _, board in BOARD_PROPERTIES.items()],
-        help="Name of the model for each Zephyr board.",
-    ),
-    server.ProjectOption(
-        "main_stack_size",
+        "config_main_stack_size",

Review comment:
       You need to adapt the PR's title to say now `config_main_stack_size` instead of `main_stack_size`. It's not so important for the PR itself but after the squash the commit title that will land into tree won't match the code, so essentially it will be wrong.




-- 
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-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org