Introduce Model Library Format export format #7533
Conversation
areusch
force-pushed
the
model-library-format
branch
from
21d51fd
to
72df6e4
Compare
* This function produces a stable on-disk representation of TVM's compiler output. * It's intended just for use with the C runtime for microTVM right now. It could be expanded for other use cases. * This PR implements the Model Library Format RFC, which ultimately is intended to support the Project Generator API (RFC forthcoming). * There may be some changes to the format without revving the version number until downstream consumers are known. The Project Generator API is the first such known downstream consumer. * There are no plans currently to support generating old Model Library Format from TVM. The version number is intended as a compatibility check between the generator and downstream consumers.
areusch
force-pushed
the
model-library-format
branch
from
72df6e4
to
bd5c6f3
Compare
There was a problem hiding this comment.
Hi @areusch ,
Thanks for this.
I could not find how crt/ sources get into the Model Library -- Maybe I have missed that, if so appreciate if you can point me where that gets implemented.
I think my concerns are mainly stemming from the fact on the dependency of the presense of json to create the memory map and would like to explore if thats a true dependency.
python/tvm/runtime/module.py
Outdated
| @@ -370,6 +370,33 @@ def export_library(self, file_name, fcompile=None, addons=None, workspace_dir=No | |||
|
|
|||
| return fcompile(file_name, files, **kwargs) | |||
|
|
|||
| def export_model_library_format(self, codegen_dir: str): | |||
There was a problem hiding this comment.
I understand that this does not support non-DSO-exportable models yet.
I think it should be better to throw a not implemented error if the current runtime.Module contains those rather than providing the illusion of successful export of Model Library format. WDYT?
There was a problem hiding this comment.
I added an error when non-c and non-llvm modules are encountered
|
|
||
| return memory_map | ||
|
|
||
| def export_model_library_format(self, file_name): |
There was a problem hiding this comment.
[Clarification] Would it be the expectation that we would need to implement a simliar function in aot_runtime_factory (or whatever the runtime family categorization we finally agree) sometime later ?
Im asking this because the current implementation seems to have a dependency on the presense of the json (which is truly derived from running relay graph plan memory on main function). So based on the anwser to the above question,
Yes -- then this is fine as the graph runtime is always going to have json and aot runtime implement this differently.
No -- then we might need to capture the sids prior to the creation of json.
Having said that, I would personally prefer to use relay.Expr --> sid map to generate the memory map.
WDYT ?
There was a problem hiding this comment.
I think the internals here are all subject to being rewritten when we broaden support. right now i'm just bolting this on until it's better understood what kind of standardized data structure should be returned from GraphPlanMemory. I agree with your insight, though--directly building this from the memory planner output would be simpler and easier to maintain. I think as we move to add e.g. tensor pinning, we can revisit this.
|
|
||
| seen_storage_ids = set() | ||
| memory_map = [] | ||
| for node_id, storage_id in enumerate(graph["attrs"]["storage_id"][1]): |
There was a problem hiding this comment.
See my comment below -- this could be simplified if we can have relay.Expr --> sid Map somewhere accessible and use that to create the json later while this function also being another consumer of that map rather than parsing the json and extracting size information out of it. WDYT?
There was a problem hiding this comment.
I agree with that, see other comment
|
@manupa-arm so I actually am wondering if the crt should go here after all--now that I've added I think I've addressed the remainder of your concerns. |
|
Yes, that sounds reasonable. LGTM. |
* Introduce Model Library Format export format. * This function produces a stable on-disk representation of TVM's compiler output. * It's intended just for use with the C runtime for microTVM right now. It could be expanded for other use cases. * This PR implements the Model Library Format RFC, which ultimately is intended to support the Project Generator API (RFC forthcoming). * There may be some changes to the format without revving the version number until downstream consumers are known. The Project Generator API is the first such known downstream consumer. * There are no plans currently to support generating old Model Library Format from TVM. The version number is intended as a compatibility check between the generator and downstream consumers.
* Introduce Model Library Format export format. * This function produces a stable on-disk representation of TVM's compiler output. * It's intended just for use with the C runtime for microTVM right now. It could be expanded for other use cases. * This PR implements the Model Library Format RFC, which ultimately is intended to support the Project Generator API (RFC forthcoming). * There may be some changes to the format without revving the version number until downstream consumers are known. The Project Generator API is the first such known downstream consumer. * There are no plans currently to support generating old Model Library Format from TVM. The version number is intended as a compatibility check between the generator and downstream consumers.
compiler output.
now. It could be expanded for other use cases.
is intended to support the Project Generator API (RFC
forthcoming).
number until downstream consumers are known. The Project Generator
API is the first such known downstream consumer.
Library Format from TVM. The version number is intended as a
compatibility check between the generator and downstream consumers.
@gromero @tom-gall @manupa-arm @leandron @u99127 @jroesch @tqchen @tmoreau89 @mdw-octoml