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/01/28 22:08:25 UTC
[GitHub] [tvm] CircleSpin opened a new pull request #7366: [TVMC] Allow manual shape specification in tvmc
CircleSpin opened a new pull request #7366:
URL: https://github.com/apache/tvm/pull/7366
Currently tvmc does not allow users to overriding specific shapes (such as batch sizes) and instead uses what is defined within the model file. In most cases this is most practical, but in there are some cases that would benefit from additional flexibility i.e. undefined shape sizes, fiddling with batch sizes. This PR adds support for manually specifying input shapes. :)
----------------------------------------------------------------
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] ekalda commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
ekalda commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-769804355
From what I can tell, the functionality this PR implements is a superset of the one in my PR, so I'm happy to close mine.
----------------------------------------------------------------
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] leandron commented on a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r568443310
##########
File path: python/tvm/driver/tvmc/frontends.py
##########
@@ -285,17 +291,18 @@ def suffixes():
# Torch Script is a zip file, but can be named pth
return ["pth", "zip"]
- def load(self, path):
+ def load(self, path, shape_dict=None):
# pylint: disable=C0415
import torch
traced_model = torch.jit.load(path)
+ traced_model.eval() # Switch to inference mode
- inputs = list(traced_model.graph.inputs())[1:]
- input_shapes = [inp.type().sizes() for inp in inputs]
+ if shape_dict is None:
+ raise TVMCException("--input-shapes must be specified for %s" % self.name())
Review comment:
A suggestion here would be moving this validation/error msg to be checked before the model is loaded (line 297 maybe?), just so that the user don't wait the time for a (sometimes big) model to be loaded, just to be presented with an error message that we could've checked already.
----------------------------------------------------------------
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] CircleSpin commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
CircleSpin commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-769432247
@leandron @jwfromm @mbrookhart
Could you take a look at this PR and let me know what you think?
----------------------------------------------------------------
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] comaniac edited a comment on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac edited a comment on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-774346429
> @masahi the previous build error seems like a torch failure unrelated to TVM but is not at all descriptive. Have you run into that one before?
It's not related to torch failure. It was because of the 2-hour timeout on CI and has been worked around.
----------------------------------------------------------------
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] leandron commented on a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r568443310
##########
File path: python/tvm/driver/tvmc/frontends.py
##########
@@ -285,17 +291,18 @@ def suffixes():
# Torch Script is a zip file, but can be named pth
return ["pth", "zip"]
- def load(self, path):
+ def load(self, path, shape_dict=None):
# pylint: disable=C0415
import torch
traced_model = torch.jit.load(path)
+ traced_model.eval() # Switch to inference mode
- inputs = list(traced_model.graph.inputs())[1:]
- input_shapes = [inp.type().sizes() for inp in inputs]
+ if shape_dict is None:
+ raise TVMCException("--input-shapes must be specified for %s" % self.name())
Review comment:
A suggestion here would be moving this validation/error msg to be checked before the model is loaded (line 297 maybe?), just so that the user don't wait the time for a (sometimes big) model to be loaded, just to be presented with an error message that we could've checked already.
----------------------------------------------------------------
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] CircleSpin commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
CircleSpin commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-771972321
Thanks for the catches you both found, @comaniac and @leandron :smiley:
----------------------------------------------------------------
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] leandron edited a comment on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
leandron edited a comment on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-776120941
@comaniac it looks like now the CI issues are gone, can we merge this one when you have a moment?
----------------------------------------------------------------
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] leandron commented on a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r568107892
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -136,3 +138,46 @@ def tracker_host_port_from_cli(rpc_tracker_str):
logger.info("RPC tracker port: %s", rpc_port)
return rpc_hostname, rpc_port
+
+
+def parse_shape_string(inputs):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
+ the desired shape for specific model inputs.
+
+ Returns
+ -------
+ shape_dict: dict
+ A dictionary mapping input names to their shape for use in relay frontend converters.
+ """
+ inputs = inputs.replace(" ", "")
+ # Check if the passed input is in the proper format.
+ valid_pattern = re.compile("(\w+:(\d+(x|X))*(\d)+)(,(\w+:(\d+(x|X))*(\d)+))*")
+ result = re.fullmatch(valid_pattern, inputs)
Review comment:
> See https://docs.python.org/3/library/argparse.html#fromfile-prefix-chars for reference
That's really cool @comaniac I didn't know this.
I agree that `data:[-1,3,224,224] data2: [10,10]` syntax looks clearer, IMHO.
----------------------------------------------------------------
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] leandron commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
leandron commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-774480100
> I don't think doing frontend tests during CPU unittest is a good idea. I suggest moving `test_frontends.py` to `test/python/frontend`.
Some test needs to be done to exercise the integration between TVMC and the underlying APIs - we want to keep this to a minimum, and will be reducing the number of tests that actually do this as per previous discussions in #6831 (cc @d-smirnov).
In #7304 I have the first test within TVMC that mocks all the APIs and just checks for arguments being passed - I intend to use that one as an example to motivate some refactoring in the TVMC tests to make them easier/faster to run.
----------------------------------------------------------------
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 #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
jwfromm commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-774403445
this works fine locally with the same version of torch and torchvision as CI. Not sure what the deal is.
----------------------------------------------------------------
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] comaniac commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-770124975
Also cc @masahi to review PyTorch shape handling.
----------------------------------------------------------------
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] CircleSpin commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
CircleSpin commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-771181762
@comaniac @leandron @jwfromm
The code has been refactored. The input syntax has been updated so that "data:[32,3,224,224] data2:[1,4,4,4]" will work, and the previous one no longer will. Could you take another pass and let me know your thoughts on this new version?
----------------------------------------------------------------
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 commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-774406152
I don't think doing frontend tests during CPU unittest is a good idea. I suggest moving `test_frontends.py` to `test/python/frontend`.
----------------------------------------------------------------
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] comaniac commented on a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r568185379
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -136,3 +138,40 @@ def tracker_host_port_from_cli(rpc_tracker_str):
logger.info("RPC tracker port: %s", rpc_port)
return rpc_hostname, rpc_port
+
+
+def parse_shape_string(inputs_string):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs_string: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
Review comment:
Update the syntax.
----------------------------------------------------------------
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 a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r568111211
##########
File path: tests/python/driver/tvmc/test_common.py
##########
@@ -149,3 +149,27 @@ def test_tracker_host_port_from_cli__only_hostname__default_port_is_9090():
assert expected_host == actual_host
assert expected_port == actual_port
+
+
+def test_shape_parser():
+ # Check that a valid input is parsed correctly
+ shape_string = "input:10x10x10"
+ shape_dict = tvmc.common.parse_shape_string(shape_string)
+ assert shape_dict == {"input": [10, 10, 10]}
+ # Check that multiple valid input shapes are parse correctly
+ shape_string = "input:10x10x10,input2:20x20x20x20"
+ shape_dict = tvmc.common.parse_shape_string(shape_string)
+ assert shape_dict == {"input": [10, 10, 10], "input2": [20, 20, 20, 20]}
+ # Check that alternate syntax parses correctly
+ shape_string = "input:10X10X10, input2:20X20X20X20"
+ shape_dict = tvmc.common.parse_shape_string(shape_string)
+ assert shape_dict == {"input": [10, 10, 10], "input2": [20, 20, 20, 20]}
+
+ # Check that invalid pattern raises expected error.
+ shape_string = "input:ax10"
+ with pytest.raises(argparse.ArgumentTypeError):
+ tvmc.common.parse_shape_string(shape_string)
+ # Check that input with invalid separators raises error.
+ shape_string = "input:5,10 input2:10,10"
+ with pytest.raises(argparse.ArgumentTypeError):
+ tvmc.common.parse_shape_string(shape_string)
Review comment:
I actually really prefer having these grouped under a single test since they're all testing functionality of the same simple function and breaking them into separate tests would add a lot of bloat for little benefit. @comaniac, which way do you prefer?
----------------------------------------------------------------
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] apivovarov edited a comment on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
apivovarov edited a comment on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-781466041
Quite often I see the following failure in CI builds
```
test_frontends.py::test_load_model__pth munmap_chunk(): invalid pointer
Fatal Python error: Aborted
```
`test_load_model__pth` was added in this PR
Issue: https://github.com/apache/tvm/issues/7471
----------------------------------------------------------------
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] leandron commented on a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r567214252
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -136,3 +138,46 @@ def tracker_host_port_from_cli(rpc_tracker_str):
logger.info("RPC tracker port: %s", rpc_port)
return rpc_hostname, rpc_port
+
+
+def parse_shape_string(inputs):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
+ the desired shape for specific model inputs.
+
+ Returns
+ -------
+ shape_dict: dict
+ A dictionary mapping input names to their shape for use in relay frontend converters.
+ """
+ inputs = inputs.replace(" ", "")
+ # Check if the passed input is in the proper format.
+ valid_pattern = re.compile("(\w+:(\d+(x|X))*(\d)+)(,(\w+:(\d+(x|X))*(\d)+))*")
+ result = re.fullmatch(valid_pattern, inputs)
+ if result is None:
+ raise argparse.ArgumentTypeError(
+ "--shapes argument must be of the form 'input_name:dim1xdim2x...xdimN,input_name2:dim1xdim2"
+ )
+ d = {}
+ # Break apart each specific input string
+ inputs = inputs.split(",")
+ for string in inputs:
+ # Split name from shape string.
+ string = string.split(":")
+ shapelist = []
+ # Separate each dimension in the shape.
+ string[1] = string[1].lower().split("x")
+ # Parse each dimension into an integer.
+ for x in string[1]:
+ x = int(x)
+ # Negative numbers are converted to dynamic axes.
+ if x < 0:
+ x = relay.Any()
+ shapelist.append(x)
+ # Assign dictionary key value pair.
+ d[string[0]] = shapelist
+ return d
Review comment:
I suggest renaming `d` to be `shape_dict`, according to the docstring on this function.
----------------------------------------------------------------
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] CircleSpin commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
CircleSpin commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-770117017
Thank you @ekalda for your super thorough feedback! :trophy: We've tried to incorporate it, and also included a test in test_frontends.py that imports a torchvision resnet18 using the new shape specification. I've also added shape inputs to tuning as you had mentioned. Could you take another look and let me know if you have any additional thoughts or feedback? :smile:
----------------------------------------------------------------
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] apivovarov edited a comment on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
apivovarov edited a comment on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-781466041
Quite often I see the following failure in CI builds
```
test_frontends.py::test_load_model__pth munmap_chunk(): invalid pointer
Fatal Python error: Aborted
```
`test_load_model__pth` was added in this PR
https://github.com/apache/tvm/issues/7471
----------------------------------------------------------------
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] comaniac commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-774176771
> I think this one is ready for merge, right? Just need another CI run...
@CircleSpin please fix or re-trigger the CI.
----------------------------------------------------------------
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] ekalda commented on a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
ekalda commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r566843771
##########
File path: python/tvm/driver/tvmc/frontends.py
##########
@@ -297,6 +303,15 @@ def load(self, path):
traced_model.eval() # Switch to inference mode
input_shapes = [("input{}".format(idx), shape) for idx, shape in enumerate(shapes)]
Review comment:
(1) That "shapes" in "enumerate(shapes)" is not defined (and old bug, something I discovered when I tried to run a PyTorch model with tvmc).
(2) This approach of extracting input shapes form PyTorch model is not functional any more (see the discussion in #7359 ), so this parameter needs to be set for PyTorch and the PyTorch frontend should throw an error when it is not set. (Unless someone maybe knows a way to extract the shapes from the model?)
##########
File path: python/tvm/driver/tvmc/compiler.py
##########
@@ -36,6 +36,41 @@
logger = logging.getLogger("TVMC")
+def parse_shape(inputs):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
+ the desired shape for specific model inputs.
+
+ Returns
+ -------
+ shape_dict: dict
+ A dictionary mapping input names to their shape for use in relay frontend converters.
+ """
+ d = {}
+ # Break apart each specific input string
+ inputs = inputs.split(",")
+ for string in inputs:
+ # Split name from shape string.
+ string = string.split(":")
+ shapelist = []
+ # Separate each dimension in the shape.
+ string[1] = string[1].split("x")
+ # Parse each dimension into an integer.
+ for x in string[1]:
+ x = int(x)
+ # Negative numbers are converted to dynamic axes.
+ if x < 0:
+ x = relay.Any()
+ shapelist.append(x)
+ # Assign dictionary key value pair.
+ d[string[0]] = shapelist
+ return d
+
Review comment:
Maybe it would be good to add some error handling when the user input is not in the desired format? Also, should we maybe allow inputs that are "close", e.g. "name:num1Xnum2" or "name:num1xnum2, name2:num1xnum2" ? Some unit tests to handle the corner cases would be nice :)
##########
File path: tests/python/driver/tvmc/test_compiler.py
##########
@@ -56,6 +53,15 @@ def test_compile_tflite_module(tflite_mobilenet_v1_1_quant):
assert type(dumps) is dict
+def test_compile_tflite_module(tflite_mobilenet_v1_1_quant):
+ # Check default compilation.
+ verify_compile_tflite_module(tflite_mobilenet_v1_1_quant)
+ # Check with manual shape override
+ shape_string = "input:1x224x224x3"
+ shape_dict = tvmc.compiler.parse_shape(shape_string)
+ verify_compile_onnx_module(tflite_mobilenet_v1_1_quant, shape_dict)
Review comment:
Should this call verify_compile_tflite_module?
----------------------------------------------------------------
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] comaniac commented on a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r567167046
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -136,3 +138,46 @@ def tracker_host_port_from_cli(rpc_tracker_str):
logger.info("RPC tracker port: %s", rpc_port)
return rpc_hostname, rpc_port
+
+
+def parse_shape_string(inputs):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
+ the desired shape for specific model inputs.
+
+ Returns
+ -------
+ shape_dict: dict
+ A dictionary mapping input names to their shape for use in relay frontend converters.
+ """
+ inputs = inputs.replace(" ", "")
+ # Check if the passed input is in the proper format.
+ valid_pattern = re.compile("(\w+:(\d+(x|X))*(\d)+)(,(\w+:(\d+(x|X))*(\d)+))*")
+ result = re.fullmatch(valid_pattern, inputs)
+ if result is None:
+ raise argparse.ArgumentTypeError(
+ "--shapes argument must be of the form 'input_name:dim1xdim2x...xdimN,input_name2:dim1xdim2"
+ )
+ d = {}
+ # Break apart each specific input string
+ inputs = inputs.split(",")
Review comment:
Do not mess up with types. `inputs` is already a string.
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -136,3 +138,46 @@ def tracker_host_port_from_cli(rpc_tracker_str):
logger.info("RPC tracker port: %s", rpc_port)
return rpc_hostname, rpc_port
+
+
+def parse_shape_string(inputs):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
+ the desired shape for specific model inputs.
+
+ Returns
+ -------
+ shape_dict: dict
+ A dictionary mapping input names to their shape for use in relay frontend converters.
+ """
+ inputs = inputs.replace(" ", "")
+ # Check if the passed input is in the proper format.
+ valid_pattern = re.compile("(\w+:(\d+(x|X))*(\d)+)(,(\w+:(\d+(x|X))*(\d)+))*")
+ result = re.fullmatch(valid_pattern, inputs)
+ if result is None:
+ raise argparse.ArgumentTypeError(
+ "--shapes argument must be of the form 'input_name:dim1xdim2x...xdimN,input_name2:dim1xdim2"
+ )
+ d = {}
+ # Break apart each specific input string
+ inputs = inputs.split(",")
+ for string in inputs:
Review comment:
Please avoid bad naming. `string` is too general and looks like a preserved word (although it's not).
##########
File path: tests/python/driver/tvmc/test_common.py
##########
@@ -149,3 +149,27 @@ def test_tracker_host_port_from_cli__only_hostname__default_port_is_9090():
assert expected_host == actual_host
assert expected_port == actual_port
+
+
+def test_shape_parser():
Review comment:
Cover negative shapes.
##########
File path: python/tvm/driver/tvmc/frontends.py
##########
@@ -389,6 +396,8 @@ def load_model(path, model_format=None):
model_format : str, optional
The underlying framework used to create the model.
If not specified, this will be inferred from the file type.
+ shape_dict : dict, optional
+ A mapping between input names and their desired shape.
Review comment:
```suggestion
Mapping from input names to their shapes.
```
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -136,3 +138,46 @@ def tracker_host_port_from_cli(rpc_tracker_str):
logger.info("RPC tracker port: %s", rpc_port)
return rpc_hostname, rpc_port
+
+
+def parse_shape_string(inputs):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
+ the desired shape for specific model inputs.
+
+ Returns
+ -------
+ shape_dict: dict
+ A dictionary mapping input names to their shape for use in relay frontend converters.
+ """
+ inputs = inputs.replace(" ", "")
+ # Check if the passed input is in the proper format.
+ valid_pattern = re.compile("(\w+:(\d+(x|X))*(\d)+)(,(\w+:(\d+(x|X))*(\d)+))*")
+ result = re.fullmatch(valid_pattern, inputs)
+ if result is None:
+ raise argparse.ArgumentTypeError(
+ "--shapes argument must be of the form 'input_name:dim1xdim2x...xdimN,input_name2:dim1xdim2"
+ )
+ d = {}
+ # Break apart each specific input string
+ inputs = inputs.split(",")
+ for string in inputs:
+ # Split name from shape string.
+ string = string.split(":")
+ shapelist = []
+ # Separate each dimension in the shape.
+ string[1] = string[1].lower().split("x")
+ # Parse each dimension into an integer.
+ for x in string[1]:
+ x = int(x)
+ # Negative numbers are converted to dynamic axes.
+ if x < 0:
+ x = relay.Any()
+ shapelist.append(x)
Review comment:
```suggestion
shapelist.append(x if x >= 0 else relay.Any())
```
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -136,3 +138,46 @@ def tracker_host_port_from_cli(rpc_tracker_str):
logger.info("RPC tracker port: %s", rpc_port)
return rpc_hostname, rpc_port
+
+
+def parse_shape_string(inputs):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
+ the desired shape for specific model inputs.
+
+ Returns
+ -------
+ shape_dict: dict
+ A dictionary mapping input names to their shape for use in relay frontend converters.
+ """
+ inputs = inputs.replace(" ", "")
+ # Check if the passed input is in the proper format.
+ valid_pattern = re.compile("(\w+:(\d+(x|X))*(\d)+)(,(\w+:(\d+(x|X))*(\d)+))*")
+ result = re.fullmatch(valid_pattern, inputs)
Review comment:
- Seems doesn't match negative numbers such as `data:-1x3x224x224`.
- Same comment as the one I left in another PR: This syntax is not straightforward to many users. I would suggest using either JSON or Python syntax.
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -136,3 +138,46 @@ def tracker_host_port_from_cli(rpc_tracker_str):
logger.info("RPC tracker port: %s", rpc_port)
return rpc_hostname, rpc_port
+
+
+def parse_shape_string(inputs):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
+ the desired shape for specific model inputs.
+
+ Returns
+ -------
+ shape_dict: dict
+ A dictionary mapping input names to their shape for use in relay frontend converters.
+ """
+ inputs = inputs.replace(" ", "")
+ # Check if the passed input is in the proper format.
+ valid_pattern = re.compile("(\w+:(\d+(x|X))*(\d)+)(,(\w+:(\d+(x|X))*(\d)+))*")
+ result = re.fullmatch(valid_pattern, inputs)
+ if result is None:
+ raise argparse.ArgumentTypeError(
+ "--shapes argument must be of the form 'input_name:dim1xdim2x...xdimN,input_name2:dim1xdim2"
+ )
+ d = {}
+ # Break apart each specific input string
+ inputs = inputs.split(",")
+ for string in inputs:
+ # Split name from shape string.
+ string = string.split(":")
+ shapelist = []
+ # Separate each dimension in the shape.
+ string[1] = string[1].lower().split("x")
Review comment:
Ditto. Do not mess up with types.
##########
File path: python/tvm/driver/tvmc/frontends.py
##########
@@ -54,13 +54,15 @@ def suffixes():
"""File suffixes (extensions) used by this frontend"""
@abstractmethod
- def load(self, path):
+ def load(self, path, shape_dict=None):
"""Load a model from a given path.
Parameters
----------
path: str
Path to a file
+ shape_dict: dict, optional
+ A dictionary mapping input names to shapes.
Review comment:
```suggestion
Mapping from input names to their shapes.
```
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -136,3 +138,46 @@ def tracker_host_port_from_cli(rpc_tracker_str):
logger.info("RPC tracker port: %s", rpc_port)
return rpc_hostname, rpc_port
+
+
+def parse_shape_string(inputs):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
+ the desired shape for specific model inputs.
+
+ Returns
+ -------
+ shape_dict: dict
+ A dictionary mapping input names to their shape for use in relay frontend converters.
+ """
+ inputs = inputs.replace(" ", "")
+ # Check if the passed input is in the proper format.
+ valid_pattern = re.compile("(\w+:(\d+(x|X))*(\d)+)(,(\w+:(\d+(x|X))*(\d)+))*")
+ result = re.fullmatch(valid_pattern, inputs)
+ if result is None:
+ raise argparse.ArgumentTypeError(
+ "--shapes argument must be of the form 'input_name:dim1xdim2x...xdimN,input_name2:dim1xdim2"
+ )
+ d = {}
Review comment:
Please avoid bad naming.
##########
File path: python/tvm/driver/tvmc/frontends.py
##########
@@ -285,17 +291,18 @@ def suffixes():
# Torch Script is a zip file, but can be named pth
return ["pth", "zip"]
- def load(self, path):
+ def load(self, path, shape_dict=None):
# pylint: disable=C0415
import torch
traced_model = torch.jit.load(path)
+ traced_model.eval() # Switch to inference mode
- inputs = list(traced_model.graph.inputs())[1:]
- input_shapes = [inp.type().sizes() for inp in inputs]
+ if shape_dict is None:
+ raise TVMCException("--shapes must be specified for {}".format(self.name()))
Review comment:
```suggestion
raise TVMCException("--shapes must be specified for %s" % self.name())
```
##########
File path: python/tvm/driver/tvmc/compiler.py
##########
@@ -158,6 +167,9 @@ def compile_model(
The layout to convert the graph to. Note, the convert layout
pass doesn't currently guarantee the whole of the graph will
be converted to the chosen layout.
+ shape_dict: dict, optional
+ A mapping between input names and their shape. This is useful
+ to override the default values in a model if needed.
Review comment:
```suggestion
A mapping from input names to their shapes. When present,
the default shapes in the model will be overwritten.
```
----------------------------------------------------------------
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] leandron commented on a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r567214252
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -136,3 +138,46 @@ def tracker_host_port_from_cli(rpc_tracker_str):
logger.info("RPC tracker port: %s", rpc_port)
return rpc_hostname, rpc_port
+
+
+def parse_shape_string(inputs):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
+ the desired shape for specific model inputs.
+
+ Returns
+ -------
+ shape_dict: dict
+ A dictionary mapping input names to their shape for use in relay frontend converters.
+ """
+ inputs = inputs.replace(" ", "")
+ # Check if the passed input is in the proper format.
+ valid_pattern = re.compile("(\w+:(\d+(x|X))*(\d)+)(,(\w+:(\d+(x|X))*(\d)+))*")
+ result = re.fullmatch(valid_pattern, inputs)
+ if result is None:
+ raise argparse.ArgumentTypeError(
+ "--shapes argument must be of the form 'input_name:dim1xdim2x...xdimN,input_name2:dim1xdim2"
+ )
+ d = {}
+ # Break apart each specific input string
+ inputs = inputs.split(",")
+ for string in inputs:
+ # Split name from shape string.
+ string = string.split(":")
+ shapelist = []
+ # Separate each dimension in the shape.
+ string[1] = string[1].lower().split("x")
+ # Parse each dimension into an integer.
+ for x in string[1]:
+ x = int(x)
+ # Negative numbers are converted to dynamic axes.
+ if x < 0:
+ x = relay.Any()
+ shapelist.append(x)
+ # Assign dictionary key value pair.
+ d[string[0]] = shapelist
+ return d
Review comment:
I suggest renaming `d` to be `shape_dict`, according to the doctoring on this function.
##########
File path: tests/python/driver/tvmc/test_common.py
##########
@@ -149,3 +149,27 @@ def test_tracker_host_port_from_cli__only_hostname__default_port_is_9090():
assert expected_host == actual_host
assert expected_port == actual_port
+
+
+def test_shape_parser():
+ # Check that a valid input is parsed correctly
+ shape_string = "input:10x10x10"
+ shape_dict = tvmc.common.parse_shape_string(shape_string)
+ assert shape_dict == {"input": [10, 10, 10]}
+ # Check that multiple valid input shapes are parse correctly
+ shape_string = "input:10x10x10,input2:20x20x20x20"
+ shape_dict = tvmc.common.parse_shape_string(shape_string)
+ assert shape_dict == {"input": [10, 10, 10], "input2": [20, 20, 20, 20]}
+ # Check that alternate syntax parses correctly
+ shape_string = "input:10X10X10, input2:20X20X20X20"
+ shape_dict = tvmc.common.parse_shape_string(shape_string)
+ assert shape_dict == {"input": [10, 10, 10], "input2": [20, 20, 20, 20]}
+
+ # Check that invalid pattern raises expected error.
+ shape_string = "input:ax10"
+ with pytest.raises(argparse.ArgumentTypeError):
+ tvmc.common.parse_shape_string(shape_string)
+ # Check that input with invalid separators raises error.
+ shape_string = "input:5,10 input2:10,10"
+ with pytest.raises(argparse.ArgumentTypeError):
+ tvmc.common.parse_shape_string(shape_string)
Review comment:
I suggest splitting these test cases above in specific unit test - I see 5 self-contained unit tests here.
##########
File path: tests/python/driver/tvmc/test_compiler.py
##########
@@ -56,6 +53,15 @@ def test_compile_tflite_module(tflite_mobilenet_v1_1_quant):
assert type(dumps) is dict
+def test_compile_tflite_module(tflite_mobilenet_v1_1_quant):
+ # Check default compilation.
Review comment:
```suggestion
def test_compile_tflite_module(tflite_mobilenet_v1_1_quant):
# some CI environments wont offer flute, so skip in case it is not present
pytest.importorskip("tflite")
```
Not all CI environments will offer `tflite`.
##########
File path: python/tvm/driver/tvmc/autotuner.py
##########
@@ -210,6 +210,13 @@ def add_tune_parser(subparsers):
# can be improved in future to add integration with a modelzoo
# or URL, for example.
parser.add_argument("FILE", help="path to the input model file")
+ parser.add_argument(
+ "--shapes",
Review comment:
Question: is the terminology `--shapes` specific enough for people to always infer they are "input shapes"? I'm asking because in the PR we previously had in discussion, we called it `--input-shapes`, and just want to hear what others think.
cc @comaniac @ekalda
----------------------------------------------------------------
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] leandron commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
leandron commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-776120941
@comaniac it looks like now the CI issue are gone, can we merge this one when you have a moment?
----------------------------------------------------------------
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 #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
jwfromm commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-769510961
Funny that these two very similar PRs landed so close to one another. After reading both, I would argue in favor of choosing this implementation. I think its important to be able to specify a mapping between input name and shape. Not only is this important for constructing a shape dictionary for all non-pytorch frontends, but it also allows a subset of shapes to be overwritten. If a bert model for example had 4 default shapes, this approach would allow us to specify `seq_len=[256]` while leaving the other 3 shapes alone.
----------------------------------------------------------------
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] comaniac commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-769522432
@CircleSpin @ekalda could you folks organize a plan to work on the one PR and close the other?
----------------------------------------------------------------
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] leandron commented on a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r567215242
##########
File path: tests/python/driver/tvmc/test_compiler.py
##########
@@ -56,6 +53,15 @@ def test_compile_tflite_module(tflite_mobilenet_v1_1_quant):
assert type(dumps) is dict
+def test_compile_tflite_module(tflite_mobilenet_v1_1_quant):
+ # Check default compilation.
Review comment:
```suggestion
def test_compile_tflite_module(tflite_mobilenet_v1_1_quant):
# some CI environments wont offer flute, so skip in case it is not present
pytest.importorskip("tflite")
```
Not all CI environments will offer `tflite`.
----------------------------------------------------------------
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] leandron commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
leandron commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-773981268
I think this one is ready for merge, right?
----------------------------------------------------------------
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 edited a comment on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
jwfromm edited a comment on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-769510961
Funny that these two very similar PRs landed so close to one another. After reading both, I would argue in favor of choosing this implementation. I think its important to be able to specify a mapping between input name and shape. Not only is this important for constructing a shape dictionary for all non-pytorch frontends, but it also allows a subset of shapes to be overwritten. If a bert model for example had 4 default shapes, this approach would allow us to specify `seq_len=[256]` while leaving the other 3 shapes alone. This would be much less clear without names.
----------------------------------------------------------------
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 a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r568093268
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -136,3 +138,46 @@ def tracker_host_port_from_cli(rpc_tracker_str):
logger.info("RPC tracker port: %s", rpc_port)
return rpc_hostname, rpc_port
+
+
+def parse_shape_string(inputs):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
+ the desired shape for specific model inputs.
+
+ Returns
+ -------
+ shape_dict: dict
+ A dictionary mapping input names to their shape for use in relay frontend converters.
+ """
+ inputs = inputs.replace(" ", "")
+ # Check if the passed input is in the proper format.
+ valid_pattern = re.compile("(\w+:(\d+(x|X))*(\d)+)(,(\w+:(\d+(x|X))*(\d)+))*")
+ result = re.fullmatch(valid_pattern, inputs)
Review comment:
@comaniac I agree the syntax is a little confusing, but discussed it with Jocelyn and there are some tradeoffs that would be made using a more Python/JSON like syntax. Since we're passing the shapes argument via command line, we cant use spaces or semicolons unless the whole argument is wrapped in quotes. For example, if we used a python like syntax such as `tvmc compile --shapes=data:-1,3,224,224 data2:10,10` the space would prevent argparse from recognizing it as a single argument. We could require quote wrapping but I personally think that is more likely to cause issues than using `x` instead of `,` to separate dimensions. Do you have a recommendation for the input format that you prefer or are you ok with always wrapping in quotes?
----------------------------------------------------------------
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] CircleSpin commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
CircleSpin commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-771097618
@hogepodge
----------------------------------------------------------------
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] comaniac commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-774346429
> @masahi the previous build error seems like a torch failure unrelated to TVM but is not at all descriptive. Have you run into that one before?
It's not related to torch failure. It was because of the 2-hour timeout and has been worked around.
----------------------------------------------------------------
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] CircleSpin commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
CircleSpin commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-771972321
Thanks for the catches you both found, @comaniac and @leandron :smiley:
----------------------------------------------------------------
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] comaniac commented on a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r567296230
##########
File path: python/tvm/driver/tvmc/autotuner.py
##########
@@ -210,6 +210,13 @@ def add_tune_parser(subparsers):
# can be improved in future to add integration with a modelzoo
# or URL, for example.
parser.add_argument("FILE", help="path to the input model file")
+ parser.add_argument(
+ "--shapes",
Review comment:
input-shapes is better.
----------------------------------------------------------------
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 #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
jwfromm commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-774395251
Looks like the new commit failed again in the same way, python aborting during construction of the torchvision model.
----------------------------------------------------------------
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 commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-774395837
Yeah looks wierd. Does it reproduce locally? Maybe CPU node has older torch and torchvision
----------------------------------------------------------------
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] apivovarov commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
apivovarov commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-781466041
Quite ofter I see the following failure in CI builds
```
test_frontends.py::test_load_model__pth munmap_chunk(): invalid pointer
Fatal Python error: Aborted
```
`test_load_model__pth` was added in this PR
https://github.com/apache/tvm/issues/7471
----------------------------------------------------------------
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] comaniac commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-776140560
Thanks @CircleSpin @ekalda @leandron @jwfromm @masahi
----------------------------------------------------------------
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] comaniac commented on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-769456612
Agree with @leandron. We can actually just keep one PR for both purposes, because PyTorch fix is more like a corner case of supporting custom shape.
----------------------------------------------------------------
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] leandron edited a comment on pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
leandron edited a comment on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-773981268
I think this one is ready for merge, right? Just need another CI run...
----------------------------------------------------------------
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] comaniac commented on a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r568116417
##########
File path: tests/python/driver/tvmc/test_common.py
##########
@@ -149,3 +149,27 @@ def test_tracker_host_port_from_cli__only_hostname__default_port_is_9090():
assert expected_host == actual_host
assert expected_port == actual_port
+
+
+def test_shape_parser():
+ # Check that a valid input is parsed correctly
+ shape_string = "input:10x10x10"
+ shape_dict = tvmc.common.parse_shape_string(shape_string)
+ assert shape_dict == {"input": [10, 10, 10]}
+ # Check that multiple valid input shapes are parse correctly
+ shape_string = "input:10x10x10,input2:20x20x20x20"
+ shape_dict = tvmc.common.parse_shape_string(shape_string)
+ assert shape_dict == {"input": [10, 10, 10], "input2": [20, 20, 20, 20]}
+ # Check that alternate syntax parses correctly
+ shape_string = "input:10X10X10, input2:20X20X20X20"
+ shape_dict = tvmc.common.parse_shape_string(shape_string)
+ assert shape_dict == {"input": [10, 10, 10], "input2": [20, 20, 20, 20]}
+
+ # Check that invalid pattern raises expected error.
+ shape_string = "input:ax10"
+ with pytest.raises(argparse.ArgumentTypeError):
+ tvmc.common.parse_shape_string(shape_string)
+ # Check that input with invalid separators raises error.
+ shape_string = "input:5,10 input2:10,10"
+ with pytest.raises(argparse.ArgumentTypeError):
+ tvmc.common.parse_shape_string(shape_string)
Review comment:
Since all of them are testing `parse_shape_string`, I prefer to put them together as it currently does. It seems not a big deal to locate the problematic corner case within this unit test when it fails.
----------------------------------------------------------------
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] comaniac commented on a change in pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#discussion_r568103662
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -136,3 +138,46 @@ def tracker_host_port_from_cli(rpc_tracker_str):
logger.info("RPC tracker port: %s", rpc_port)
return rpc_hostname, rpc_port
+
+
+def parse_shape_string(inputs):
+ """Parse an input shape dictionary string to a usable dictionary.
+
+ Parameters
+ ----------
+ inputs: str
+ A string of the form "name:num1xnum2x...xnumN,name2:num1xnum2xnum3" that indicates
+ the desired shape for specific model inputs.
+
+ Returns
+ -------
+ shape_dict: dict
+ A dictionary mapping input names to their shape for use in relay frontend converters.
+ """
+ inputs = inputs.replace(" ", "")
+ # Check if the passed input is in the proper format.
+ valid_pattern = re.compile("(\w+:(\d+(x|X))*(\d)+)(,(\w+:(\d+(x|X))*(\d)+))*")
+ result = re.fullmatch(valid_pattern, inputs)
Review comment:
To be honest, I'm totally fine with quotes in the command line, and we actually use it in TVMC already. For example, when specifying target, you cannot avoid using quotes for targets like `llvm -mcpu=core-axv2`. In fact, although we haven't fully landed the new target, the ultimate goal is to get rid of plain string target format and support JSON format. IMHO, it would be better if the format/syntax are consistent.
Meanwhile, if having quotes in the command line is a concern, it might be worthwhile to consider taking a command line file. See https://docs.python.org/3/library/argparse.html#fromfile-prefix-chars for reference. In this case, we can have:
```
# shapes.txt
--input-shapes
data:[-1,3,224,224] data2: [10,10]
```
```
tvmc compile @shapes.txt
```
----------------------------------------------------------------
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 #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
jwfromm commented on pull request #7366:
URL: https://github.com/apache/tvm/pull/7366#issuecomment-774342206
@masahi the previous build error seems like a torch failure unrelated to TVM but is not at all descriptive. Have you run into that one before?
----------------------------------------------------------------
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] comaniac merged pull request #7366: [TVMC] Allow manual shape specification in tvmc
Posted by GitBox <gi...@apache.org>.
comaniac merged pull request #7366:
URL: https://github.com/apache/tvm/pull/7366
----------------------------------------------------------------
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