[GitHub] [tvm] gromero commented on a change in pull request #9663: [TVMC][MicroTVM] Fix tvmc micro `project_dir` arg relative path

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

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



##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -236,18 +236,26 @@ def drive_micro(args):
     args.subcommand_handler(args)
 
 
+def get_project_dir(args: argparse.Namespace) -> str:

Review comment:
       That function will be used  in other contexts too, beyond `tvmc micro`. For instance, in `tvmc run ---device micro ...`, hence it should be moved to `common.py` and should not rely on any specific `Namespace` attribute, so it must accept a `pathlib.Path` or simply a `str` type.

##########
File path: tests/micro/common/test_tvmc.py
##########
@@ -66,13 +67,20 @@ def test_tvmc_exist(board):
 
 
 @tvm.testing.requires_micro
-def test_tvmc_model_build_only(board):
+@pytest.mark.parametrize(
+    "output_dir,",
+    [pathlib.Path("./tvmc_relative_path_test"), pathlib.Path(tempfile.mkdtemp())],
+)
+def test_tvmc_model_build_only(board, output_dir):
     target, platform = _get_target_and_platform(board)
 
+    if not os.path.isabs(output_dir):

Review comment:
       This check is actually defeating the main purpose of this test regarding relative paths since `project_dir` derives from `output_dir`  that is resolved here (made absolute here), so `project_dir`ends up not to be relative anymore. This needs to get fixed.  Only as an example and reference, a change like that on top of this PR makes the test to stress the relative paths:
   
   ```
   diff --git a/tests/micro/common/test_tvmc.py b/tests/micro/common/test_tvmc.py
   index 200c7f616..419dd9405 100644
   --- a/tests/micro/common/test_tvmc.py
   +++ b/tests/micro/common/test_tvmc.py
   @@ -110,7 +110,7 @@ def test_tvmc_model_build_only(board, output_dir):
        create_project_cmd = [
            "micro",
            "create-project",
   -        project_dir,
   +        "./project",
            tar_path,
            platform,
            "--project-option",
   @@ -123,11 +123,11 @@ def test_tvmc_model_build_only(board, output_dir):
        assert cmd_result == 0, "tvmc micro failed in step: create-project"
    
        cmd_result = _run_tvmc(
   -        ["micro", "build", project_dir, platform, "--project-option", f"{platform}_board={board}"]
   +        ["micro", "build", "./project", platform, "--project-option", f"{platform}_board={board}"]
        )
        assert cmd_result == 0, "tvmc micro failed in step: build"
        shutil.rmtree(output_dir)
   -
   +    shutil.rmtree("./project")
    
    @pytest.mark.requires_hardware
    @tvm.testing.requires_micro
   @@ -174,7 +174,7 @@ def test_tvmc_model_run(board, output_dir):
        create_project_cmd = [
            "micro",
            "create-project",
   -        project_dir,
   +        "./project",
            tar_path,
            platform,
            "--project-option",
   @@ -187,12 +187,12 @@ def test_tvmc_model_run(board, output_dir):
        assert cmd_result == 0, "tvmc micro failed in step: create-project"
    
        cmd_result = _run_tvmc(
   -        ["micro", "build", project_dir, platform, "--project-option", f"{platform}_board={board}"]
   +        ["micro", "build", "./project", platform, "--project-option", f"{platform}_board={board}"]
        )
        assert cmd_result == 0, "tvmc micro failed in step: build"
    
        cmd_result = _run_tvmc(
   -        ["micro", "flash", project_dir, platform, "--project-option", f"{platform}_board={board}"]
   +        ["micro", "flash", "./project", platform, "--project-option", f"{platform}_board={board}"]
        )
        assert cmd_result == 0, "tvmc micro failed in step: flash"
    
   @@ -201,7 +201,7 @@ def test_tvmc_model_run(board, output_dir):
                "run",
                "--device",
                "micro",
   -            project_dir,
   +            "./project",
                "--project-option",
                f"{platform}_board={board}",
                "--fill-mode",
   @@ -210,6 +210,7 @@ def test_tvmc_model_run(board, output_dir):
        )
        assert cmd_result == 0, "tvmc micro failed in step: run"
        shutil.rmtree(output_dir)
   +    shutil.rmtree("./project")
    
    
    if __name__ == "__main__":
   ```
   

##########
File path: tests/micro/common/test_tvmc.py
##########
@@ -118,17 +126,25 @@ def test_tvmc_model_build_only(board):
         ["micro", "build", project_dir, platform, "--project-option", f"{platform}_board={board}"]
     )
     assert cmd_result == 0, "tvmc micro failed in step: build"
+    shutil.rmtree(output_dir)
 
 
 @pytest.mark.requires_hardware
 @tvm.testing.requires_micro
-def test_tvmc_model_run(board):
+@pytest.mark.parametrize(
+    "output_dir,",
+    [pathlib.Path("./tvmc_relative_path_test"), pathlib.Path(tempfile.mkdtemp())],
+)
+def test_tvmc_model_run(board, output_dir):
     target, platform = _get_target_and_platform(board)
 
+    if not os.path.isabs(output_dir):

Review comment:
       Same here: it defeats the main purpose of testing relative paths as mentioned above. It need to get fixed.




-- 
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