You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/05/27 20:52:47 UTC

[GitHub] [tvm] elvin-n opened a new pull request #8153: Fix tvmc for cases when uTMV is not enabled

elvin-n opened a new pull request #8153:
URL: https://github.com/apache/tvm/pull/8153


   TVMC was changed to load uTVM python/tvm/micro/session.py that leads to checking if USE_MICRO is enabled during the build of tvm library. This is not required for other platforms, moved import to correspondent seciont


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

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



[GitHub] [tvm] masahi merged pull request #8153: Fix tvmc for cases when uTMV is not enabled

Posted by GitBox <gi...@apache.org>.
masahi merged pull request #8153:
URL: https://github.com/apache/tvm/pull/8153


   


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

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



[GitHub] [tvm] elvin-n commented on pull request #8153: Fix tvmc for cases when uTMV is not enabled

Posted by GitBox <gi...@apache.org>.
elvin-n commented on pull request #8153:
URL: https://github.com/apache/tvm/pull/8153#issuecomment-850497395


   @leandron could you please review?


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

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



[GitHub] [tvm] gromero commented on pull request #8153: Fix tvmc for cases when uTMV is not enabled

Posted by GitBox <gi...@apache.org>.
gromero commented on pull request #8153:
URL: https://github.com/apache/tvm/pull/8153#issuecomment-850560352


   @elvin-n just a heads up in case you forgot, but the last commit pushed to the CI still have the title wrong (uTMV).
   


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

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



[GitHub] [tvm] gromero commented on a change in pull request #8153: Fix tvmc for cases when uTMV is not enabled

Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #8153:
URL: https://github.com/apache/tvm/pull/8153#discussion_r641602860



##########
File path: python/tvm/driver/tvmc/model.py
##########
@@ -280,7 +284,10 @@ def export_package(
                 executor_factory, package_path, cross, cross_options, output_format
             )
         elif output_format == "mlf":
-            package_path = export_model_library_format(executor_factory, package_path)
+            if export_model_library_format is not None:

Review comment:
       I would simplify this as the following, like in other parts of the code:
   
   ```
   -            if export_model_library_format is not None:
   +            if export_model_library_format:
   ```

##########
File path: python/tvm/driver/tvmc/model.py
##########
@@ -280,7 +284,10 @@ def export_package(
                 executor_factory, package_path, cross, cross_options, output_format
             )
         elif output_format == "mlf":
-            package_path = export_model_library_format(executor_factory, package_path)
+            if export_model_library_format is not None:
+                package_path = export_model_library_format(executor_factory, package_path)
+            else:
+                raise Exception("micro tvm cannot be loaded, verify USE_MICRO in config.cmake")

Review comment:
       I think it doesn't hurt to be more explicit on what we think is wrong, so I'd like to use the same message as in `session.py`, i.e.:
   
   ```
   ImportError: micro tvm is not enabled. Set USE_MICRO to ON in config.cmake
   ```




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

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



[GitHub] [tvm] elvin-n commented on a change in pull request #8153: Fix tvmc for cases when uTMV is not enabled

Posted by GitBox <gi...@apache.org>.
elvin-n commented on a change in pull request #8153:
URL: https://github.com/apache/tvm/pull/8153#discussion_r641632965



##########
File path: python/tvm/driver/tvmc/model.py
##########
@@ -280,7 +284,10 @@ def export_package(
                 executor_factory, package_path, cross, cross_options, output_format
             )
         elif output_format == "mlf":
-            package_path = export_model_library_format(executor_factory, package_path)
+            if export_model_library_format is not None:

Review comment:
       fixed

##########
File path: python/tvm/driver/tvmc/model.py
##########
@@ -280,7 +284,10 @@ def export_package(
                 executor_factory, package_path, cross, cross_options, output_format
             )
         elif output_format == "mlf":
-            package_path = export_model_library_format(executor_factory, package_path)
+            if export_model_library_format is not None:
+                package_path = export_model_library_format(executor_factory, package_path)
+            else:
+                raise Exception("micro tvm cannot be loaded, verify USE_MICRO in config.cmake")

Review comment:
       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.

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



[GitHub] [tvm] elvin-n commented on a change in pull request #8153: Fix tvmc for cases when uTMV is not enabled

Posted by GitBox <gi...@apache.org>.
elvin-n commented on a change in pull request #8153:
URL: https://github.com/apache/tvm/pull/8153#discussion_r641630740



##########
File path: python/tvm/driver/tvmc/model.py
##########
@@ -280,7 +284,10 @@ def export_package(
                 executor_factory, package_path, cross, cross_options, output_format
             )
         elif output_format == "mlf":
-            package_path = export_model_library_format(executor_factory, package_path)
+            if export_model_library_format is not None:
+                package_path = export_model_library_format(executor_factory, package_path)
+            else:
+                raise Exception("micro tvm cannot be loaded, verify USE_MICRO in config.cmake")

Review comment:
       I would like to re-throw an original exception, but would not like to complicate and store one more attribute in global scope. Ok, let's to copy-past message

##########
File path: python/tvm/driver/tvmc/model.py
##########
@@ -280,7 +284,10 @@ def export_package(
                 executor_factory, package_path, cross, cross_options, output_format
             )
         elif output_format == "mlf":
-            package_path = export_model_library_format(executor_factory, package_path)
+            if export_model_library_format is not None:
+                package_path = export_model_library_format(executor_factory, package_path)
+            else:
+                raise Exception("micro tvm cannot be loaded, verify USE_MICRO in config.cmake")

Review comment:
       I would like to re-throw an original exception, but would not like to complicate and store one more attribute in global scope. Ok, let's copy-past message




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

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



[GitHub] [tvm] jwfromm commented on pull request #8153: Fix tvmc for cases when uTMV is not enabled

Posted by GitBox <gi...@apache.org>.
jwfromm commented on pull request #8153:
URL: https://github.com/apache/tvm/pull/8153#issuecomment-850543451


   Thanks for making this change @elvin-n, this is definitely better behavior for usrs.


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

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



[GitHub] [tvm] elvin-n commented on pull request #8153: Fix tvmc for cases when uTMV is not enabled

Posted by GitBox <gi...@apache.org>.
elvin-n commented on pull request #8153:
URL: https://github.com/apache/tvm/pull/8153#issuecomment-850693857


   > @elvin-n just a heads up in case you forgot, but the last commit pushed to the CI still has the title wrong (uTMV).
   
   changed


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

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



[GitHub] [tvm] gromero commented on a change in pull request #8153: Fix tvmc for cases when uTMV is not enabled

Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #8153:
URL: https://github.com/apache/tvm/pull/8153#discussion_r641633796



##########
File path: python/tvm/driver/tvmc/model.py
##########
@@ -280,7 +284,10 @@ def export_package(
                 executor_factory, package_path, cross, cross_options, output_format
             )
         elif output_format == "mlf":
-            package_path = export_model_library_format(executor_factory, package_path)
+            if export_model_library_format is not None:
+                package_path = export_model_library_format(executor_factory, package_path)
+            else:
+                raise Exception("micro tvm cannot be loaded, verify USE_MICRO in config.cmake")

Review comment:
       @elvin-n Yeah, I meant only copy-past the message, not the exception type.




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

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



[GitHub] [tvm] gromero edited a comment on pull request #8153: Fix tvmc for cases when uTMV is not enabled

Posted by GitBox <gi...@apache.org>.
gromero edited a comment on pull request #8153:
URL: https://github.com/apache/tvm/pull/8153#issuecomment-850560352


   @elvin-n just a heads up in case you forgot, but the last commit pushed to the CI still has the title wrong (uTMV).
   


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

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