Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TVMC] Allow direct numpy inputs to run_module #7788

Merged
merged 4 commits into from
Apr 3, 2021

Conversation

CircleSpin
Copy link
Contributor

When using TVMC from python it more convenient to pass numpy arrays as inputs rather than saved files. This PR moves the file reading into drive_run to maintain the existing behavior for command line TVMC while making the python API cleaner.

@CircleSpin
Copy link
Contributor Author

@jwfromm @leandron @comaniac @mdw-octoml

Thoughts?

@@ -230,8 +235,8 @@ def make_inputs_dict(inputs_file, shape_dict, dtype_dict, fill_mode):

Parameters
----------
inputs_file : str
Path to a .npz file containing the inputs.
inputs : dict
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
inputs : dict
inputs : dict, or None

I'm actually not quite sure how this type should be in numpy-style...in Python style I'll just write Optional[Dict], but an argument with this type is usually inputs=None. Meanwhile, a function called "make_inputs_dict" but having inputs=None is also weird...

Another direction is keeping inputs to be dict, and it becomes user's resposibility to pass {} when no inputs. In this case, you don't need the None checker.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Cody,
Thanks for the feedback. Just pushed an updated version as you suggested.

Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jwfromm jwfromm merged commit 1c1a2b5 into apache:main Apr 3, 2021
@jwfromm
Copy link
Contributor

jwfromm commented Apr 3, 2021

Thanks @CircleSpin and @comaniac. This is now merged.

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
* progress, graph params need to figure out

* black and lint

* change np.load(inputs_file) to happen in drive_run

* make inputs optional

Co-authored-by: Jocelyn <jocelyn@pop-os.localdomain>
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
* progress, graph params need to figure out

* black and lint

* change np.load(inputs_file) to happen in drive_run

* make inputs optional

Co-authored-by: Jocelyn <jocelyn@pop-os.localdomain>
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
* progress, graph params need to figure out

* black and lint

* change np.load(inputs_file) to happen in drive_run

* make inputs optional

Co-authored-by: Jocelyn <jocelyn@pop-os.localdomain>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request May 11, 2021
* progress, graph params need to figure out

* black and lint

* change np.load(inputs_file) to happen in drive_run

* make inputs optional

Co-authored-by: Jocelyn <jocelyn@pop-os.localdomain>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants