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.

Sign up for GitHub

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

Jump to bottom

Improved MLF to contain workspace info #7938

Merged
merged 5 commits into from
May 7, 2021
Merged

Improved MLF to contain workspace info #7938

merged 5 commits into from
May 7, 2021

Conversation

manupak
Copy link
Contributor

@manupak manupak commented Apr 28, 2021 •
edited
Loading

Improving the Model Library Format (MLF) 's metadata.json include workspace size information.

Added functionality to calculate workspace by each primfunc and main function. Additionally, the memory taken by constants and IO is also reported -- in case an executor want to copy them to workspace. Moreover, the workspace information required by each primfunc and main is reported in metadata.json in the Model Library Format(MLF).

cc : @areusch @giuseros @jcf94 @tqchen @Mousius

areusch
areusch requested changes May 3, 2021
View reviewed changes
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

thanks @manupa-arm , left a few comments and questions from my side

src/relay/backend/graph_executor_codegen.cc Show resolved Hide resolved
src/relay/backend/graph_executor_codegen.cc Show resolved Hide resolved
src/relay/backend/graph_executor_codegen.cc Outdated Show resolved Hide resolved
src/relay/backend/graph_executor_codegen.cc Outdated Show resolved Hide resolved
src/relay/backend/utils.cc Outdated Show resolved Hide resolved
tests/python/unittest/test_micro_model_library_format.py Outdated Show resolved Hide resolved
@areusch areusch added the status: need update need update based on feedbacks label May 3, 2021
@manupak
Copy link
Contributor Author

manupak commented May 5, 2021

Hi @areusch ,

Thanks for the review.
I've addressed your comments and answered questions. PTAL when you have some time.

@manupak
Improved MLF to contain workspace info
b8f434f 
Added functionality to calculate workspace, io and constant
memory required by each primfunc and main function. Moreover,
the workspace information required by each primfunc and main
is reported in metadata.json in the Model Library Format(MLF).
- added functionality to record tir and relay primfuncs
- added tests for model_library_format changes

Change-Id: Ib4a8b787345aa35f8a1645e8a648fad84de37bce
areusch
areusch reviewed May 5, 2021
View reviewed changes
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

cool, i think this is almost there. let's also bump the version number in Model Library Format, since we are making a change.

src/relay/backend/graph_executor_codegen.cc Outdated Show resolved Hide resolved
src/relay/backend/utils.cc Outdated Show resolved Hide resolved
@manupak
Improved MLF to contain workspace info
d96c0ac 
* disable AoT for now
* addressing comments

Change-Id: I5f041ec461b02dac6ea9c96ea50eb400d55eef53
@manupak
Copy link
Contributor Author

manupak commented May 6, 2021

Hi @areusch,

I addressed one comment but I still believe there is a value in retaining the constructor in the python for FFI passable object -- but not stressing that we absolutely need it though good to have. Let me know what you think.

areusch
areusch requested changes May 6, 2021
View reviewed changes
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

ok great. a few last things, otherwise LGTM. just since there are a number of parallel efforts, want to make sure we document when we'll be changing what for both my and others' sake :)

src/relay/backend/utils.cc Outdated Show resolved Hide resolved
src/relay/backend/graph_executor_codegen.cc Outdated Show resolved Hide resolved
python/tvm/micro/model_library_format.py Outdated Show resolved Hide resolved
@manupak
Improved MLF to contain workspace info
b7297cd 
* addressed comments
* added aot executor support

Change-Id: I9b54a7939d8ccb3c6ce0454f0fe62866ac66eb5c
@manupak
Improved MLF to contain workspace info
324626c 
* removed redundant utils.py

Change-Id: I256dd88fab31a595bf9509bd1c4ab59b0c145b1e
@manupak
Improved MLF to contain workspace info
b9330a2 
* removed redundant ffi api

Change-Id: I9ad6795aa839edfdfd05b902d4531fb0a20e894d
@manupak
Copy link
Contributor Author

manupak commented May 6, 2021

I think I've addessed the comments, PTAL

areusch
areusch approved these changes May 6, 2021
View reviewed changes
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

thanks @manupa-arm !

@manupak
Copy link
Contributor Author

manupak commented May 7, 2021

Merge?

@areusch areusch merged commit c069049 into apache:main May 7, 2021
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request May 19, 2021
@manupak @mehrdadh
Improved MLF to contain workspace info (apache#7938)
495f034 
* Improved MLF to contain workspace info

Added functionality to calculate workspace, io and constant
memory required by each primfunc and main function. Moreover,
the workspace information required by each primfunc and main
is reported in metadata.json in the Model Library Format(MLF).
- added functionality to record tir and relay primfuncs
- added tests for model_library_format changes

Change-Id: Ib4a8b787345aa35f8a1645e8a648fad84de37bce

* Improved MLF to contain workspace info

* disable AoT for now
* addressing comments

Change-Id: I5f041ec461b02dac6ea9c96ea50eb400d55eef53

* Improved MLF to contain workspace info

* addressed comments
* added aot executor support

Change-Id: I9b54a7939d8ccb3c6ce0454f0fe62866ac66eb5c

* Improved MLF to contain workspace info

* removed redundant utils.py

Change-Id: I256dd88fab31a595bf9509bd1c4ab59b0c145b1e

* Improved MLF to contain workspace info

* removed redundant ffi api

Change-Id: I9ad6795aa839edfdfd05b902d4531fb0a20e894d
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jun 17, 2021
@manupak
Improved MLF to contain workspace info (apache#7938)
094deb7 
* Improved MLF to contain workspace info

Added functionality to calculate workspace, io and constant
memory required by each primfunc and main function. Moreover,
the workspace information required by each primfunc and main
is reported in metadata.json in the Model Library Format(MLF).
- added functionality to record tir and relay primfuncs
- added tests for model_library_format changes

Change-Id: Ib4a8b787345aa35f8a1645e8a648fad84de37bce

* Improved MLF to contain workspace info

* disable AoT for now
* addressing comments

Change-Id: I5f041ec461b02dac6ea9c96ea50eb400d55eef53

* Improved MLF to contain workspace info

* addressed comments
* added aot executor support

Change-Id: I9b54a7939d8ccb3c6ce0454f0fe62866ac66eb5c

* Improved MLF to contain workspace info

* removed redundant utils.py

Change-Id: I256dd88fab31a595bf9509bd1c4ab59b0c145b1e

* Improved MLF to contain workspace info

* removed redundant ffi api

Change-Id: I9ad6795aa839edfdfd05b902d4531fb0a20e894d
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jun 17, 2021
@manupak @trevor-m
Improved MLF to contain workspace info (apache#7938)
4e72623 
* Improved MLF to contain workspace info

Added functionality to calculate workspace, io and constant
memory required by each primfunc and main function. Moreover,
the workspace information required by each primfunc and main
is reported in metadata.json in the Model Library Format(MLF).
- added functionality to record tir and relay primfuncs
- added tests for model_library_format changes

Change-Id: Ib4a8b787345aa35f8a1645e8a648fad84de37bce

* Improved MLF to contain workspace info

* disable AoT for now
* addressing comments

Change-Id: I5f041ec461b02dac6ea9c96ea50eb400d55eef53

* Improved MLF to contain workspace info

* addressed comments
* added aot executor support

Change-Id: I9b54a7939d8ccb3c6ce0454f0fe62866ac66eb5c

* Improved MLF to contain workspace info

* removed redundant utils.py

Change-Id: I256dd88fab31a595bf9509bd1c4ab59b0c145b1e

* Improved MLF to contain workspace info

* removed redundant ffi api

Change-Id: I9ad6795aa839edfdfd05b902d4531fb0a20e894d
@junrushao junrushao mentioned this pull request Nov 1, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@areusch areusch areusch approved these changes

Assignees
No one assigned
Labels
status: need update need update based on feedbacks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@manupak @areusch