gromero commented on a change in pull request #8331:
URL: https://github.com/apache/tvm/pull/8331#discussion_r658302581
##########
File path: tests/python/driver/tvmc/conftest.py
##########
@@ -167,40 +148,17 @@ def onnx_mnist():
return model_file
-@pytest.fixture(scope="session")
-def tflite_compiled_model(tmpdir_factory):
-
- # Not all CI environments will have TFLite installed
- # so we need to safely skip this fixture that will
- # crash the tests that rely on it.
- # As this is a pytest.fixture, we cannot take advantage
- # of pytest.importorskip. Using the block below instead.
- try:
- import tflite
- except ImportError:
- print("Cannot import tflite, which is required by
tflite_compiled_module_as_tarfile.")
- return ""
-
- target_dir = tmpdir_factory.mktemp("data")
- return get_sample_compiled_module(target_dir, "mock.tar")
-
-
-@pytest.fixture(scope="session")
-def tflite_compiled_model_mlf(tmpdir_factory):
+@pytest.fixture
+def tflite_tvmc_compiler(tmpdir_factory):
Review comment:
How about renaming the `tflite_tvmc_compiler` fixture to
`tflite_compile_model`, just because I don't think that `tvmc` in there is
informative. But not strong feelings about it. Feel free also get more input
from Leandro before sending a v2.
##########
File path: python/tvm/driver/tvmc/model.py
##########
@@ -332,8 +333,13 @@ def import_package(self, package_path: str):
# Model Library Format (MLF)
self.lib_name = None
self.lib_path = None
+ with open(temp.relpath("metadata.json")) as metadata_json:
+ metadata = json.load(metadata_json)
- graph = temp.relpath("runtime-config/graph/graph.json")
+ if "graph" in metadata["runtimes"]:
+ graph = temp.relpath("runtime-config/graph/graph.json")
+ else:
+ graph = None
Review comment:
Nice. I just think a hint about AOT should exist here. How about adding
a comment above `graph = None`, like "AOT runtime"?
##########
File path: tests/python/driver/tvmc/test_mlf.py
##########
@@ -82,9 +85,13 @@ def
test_tvmc_export_package_mlf(tflite_mobilenet_v1_1_quant, tmpdir_factory):
assert str(exp.value) == expected_reason, on_error
-def test_tvmc_import_package_mlf(tflite_compiled_model_mlf):
+def test_tvmc_import_package_mlf(tflite_mobilenet_v1_1_quant,
tflite_tvmc_compiler):
Review comment:
I think we can rename `test_tvmc_import_package_mlf` to
`test_tvmc_import_package_mlf_graph`.
##########
File path: tests/python/driver/tvmc/test_mlf.py
##########
@@ -97,3 +104,27 @@ def test_tvmc_import_package_mlf(tflite_compiled_model_mlf):
assert tvmc_package.graph is not None, ".graph must be set in the MLF
archive."
Review comment:
The error message must be adapted here, adding "[...] if not AOT". Maybe:
`".graph must be set in the MLF archive if not AOT runtime"
--
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:
us...@infra.apache.org
