Add "operator" style to Model Library Format #8072

areusch · 2021-05-18T19:59:15Z

This PR adds Model Library Format support to artifacts created by tvm.build. It introduces a style key to the Model Library Format metadata with two initial values:

full-model
operator

Implementations that use Model Library Format will now need to check style before reading other files in the archive. full-model indicates the previously-used format.

operator is introduced to allow exporting TVM libraries that contain only operator functions with no model-level information (e.g. executor configuration, model-wide memory planning, etc). The goal of operator style is to allow exporting fragments of models (e.g. individual TVM operators) for use with the TVM RPC Server. After the Project API refactor lands, TVM auto-tuning will produce MLF in operator style, and those MLF archives will be given to project generators with the ultimate goal of flashing and timing those operators on-device.

MLF archives with operator style contain:

codegen directory, organized as in full-model
metadata.json of the same format as full-model with different values.
src/tir-<device_type>.txt, containing pretty-printed TIR

Notably, the memory key in metadata.json contains shape information for each operator function parameter. The shape information has names correlated with those used in the TIR sources in src/tir-*.txt.

@leandron @manupa-arm @giuseros @Mousius @gromero @mehrdadh @stoa

areusch · 2021-05-25T18:14:40Z

friendly ping! this one is blocking Project API work

giuseros

Hi @areusch , sorry for the latency. As I said my github notifications stopped working for a while :)

giuseros · 2021-05-25T22:34:39Z

python/tvm/micro/model_library_format.py

+
+
+def _export_graph_model_library_format(
+    mod: executor_factory.GraphExecutorFactoryModule, tempdir: pathlib.Path


Shouldn't this be ExecutorFactoryModule to be compatible with AOT as well?

giuseros · 2021-05-25T22:38:13Z

src/printer/model_library_format_printer.cc

+        if (text_printer_.GetVarName(args[0], &var_name)) {
+          *rv = var_name;
+        }


Should this ICHECK if GetVarName returns false?

i guess the thinking is that the caller can decide what to do. since the function will return None, it should be fairly straightforward. we could raise an exception. are you thinking that users may not check the return value?

giuseros · 2021-05-25T22:45:15Z

python/tvm/micro/model_library_format.py

+        with open(src_dir / f"tir-{target_device_type}.txt", "w") as f:
+            f.write(printer["print"](ir_mod))


I am not following why adding the TIR in the archive. Is this for test purposes?

it's mostly an analogy to adding the relay.txt into the archive--to provide TVM source code for the generated code. though I see your point that TIR is quite close to the generated code.

manupak

Sorry for the delay!

I think we might want to add to title/description the PR that this is essentially adding MLF as an output format for tvm.build.

Most of comments are related to code locations and usage of runtime.Module over Object/Node. See if you agree.

manupak · 2021-05-26T08:22:55Z

src/printer/model_library_format_printer.cc

+namespace tvm {
+namespace printer {
+
+class ModelLibraryFormatPrinter : public ::tvm::runtime::ModuleNode {


Why are we extending runtime.Module ?
Any reason why we cant use Objects and Nodes to expose this to python?

we need GetFunction to expose an object to Python with member functions, and i think the member function style provides a bit more structure to the API than just e.g. placing it all in a tuple. I don't think we can use Node since it doesn't provide the sptr_to_self

I was actually referring to other objects with member functions.
E.g. :
https://github.com/manupa-arm/incubator-tvm/blob/master/src/relay/analysis/call_graph.cc
https://github.com/manupa-arm/incubator-tvm/blob/master/python/tvm/relay/analysis/call_graph.py

There are similiar structure in AutoScheduler as well. I always thought this was better than extending runtime.Modules and using packed functions. What do you think?

hmm. i think that this way is a more codified dispatch mechanism (e.g. string table lookup) than that used by Module (GetFunction typically implemented with a series of if statements and closures), but it requires additional duplication of each member function in e.g. Python. I think a proper interface-oriented FFI would just wrap all of this stuff automatically.

my preference with interface FFI functions in TVM is to use Module in general, but add some automation to the build to auto-generate GetFunction and avoid closures in that function. It is a bit weird, I'll admit, since it re-uses a mechanism meant for the runtime. But, I think it's the only generic member-function lookup we have right now, and there are some non-runtime use cases.

Yes, I think there is precedence for both approaches. Lets not block this based on this :).
Maybe its better to converge to a single policy when exposing member functions across the FFI.
cc : @tqchen @jroesch .

Though, if the intention is of this class to be used in python, I'd still prefer to have a documented interface in python -- it seems much easier to follow than the indirections via GetFunction.

manupak · 2021-05-26T08:27:04Z

python/tvm/micro/model_library_format.py

+    _populate_codegen_dir(mod, codegen_dir)
+
+
+ExportableModule = typing.Union[


Not sure whether the model_library_format.py is the right place to hold this

i guess this is a bit specific to Model Library Format--you can build shared libraries from things we don't know how to export into MLF. happy to change the name, or we can revisit this when we promote MLF to a top-level TVM export format.

Ack, sounds good for now then.

manupak · 2021-05-26T08:33:35Z

python/tvm/micro/model_library_format.py

+    memory_map = {}
+    for target_device_type, target in targets.items():
+        ir_mod = ir_module_by_target[target]
+        printer = get_global_func("tir.ModelLibraryFormatPrinter")(False, None, False)


I feel we can have this neatly hidden under _ffi_api.py and move the c++ implementations related to ModelLibraryFormatPrinter to a matching model_library_format.cc.

Why do we think ModelLibraryFormatPrinter belongs to the namespace of tir?

yeah good point. in src/printer, we have a few entry points:

src/printer/tvmscript_printer.cc defines script.AsTVMScript

src/printer/text_printer.cc defines ir.PrettyPrint and ir.AsText

so i guess the folder doesn't provide any namespace grouping right now, even though printer implementations are consolidated there. i'm okay moving to micro.ModelLibraryFormatPrinter or ir.ModelLibraryFormatPrinter, if that's what you're suggesting. tir seemed like a fit since that's how we are using it now, though it should work with any IRModule.

could you let me know which namespace you're suggesting to move to?

I was thinking of "micro.model_library_format.printer" being the registration and make printer a python function that binds to C++ under _ffi_api.py (similiar to how its done in CallGraph).

in this case, we need the member function to retrieve the mapping--this is why i used Module. as for the namespace, i don't have a strong opinion, but the only micro directory we have in src is src/runtime, and this is clearly not a runtime component. so we'd need to create src/micro, is all. i'm not opposed to that, but was following convention for Printer in keeping ModelLibraryFormatPrinter underneath src/printer, is all.

manupak · 2021-05-26T08:34:37Z

src/printer/model_library_format_printer.cc

+  TextPrinter text_printer_;
+};
+
+TVM_REGISTER_GLOBAL("tir.ModelLibraryFormatPrinter")


See my comment above about the source code arrangement

This seems fine as it works for all TIR -- I just got reminded that we moved MLF to micro for a different reason.

areusch · 2021-06-21T15:04:16Z

@giuseros @manupa-arm please take a look, i think I've addressed your comments or replied on thread

manupak

Ack -- we could use this namespace as it works for all TIR.
(It was a bit of puzzle to me why we moved MLF to micro in the first place :) )

I still think having a python class for a python exposed interface is more readable (though is more overhead in-terms of code -- yes it'd be nice if we could have a generation mechanism).

However, we should have a policy as this is a common occurrence in the codebase -- both approaches. @tqchen @jroesch.

manupak · 2021-06-21T17:19:02Z

src/printer/model_library_format_printer.cc

+    return doc.str();
+  }
+
+  PackedFunc GetFunction(const std::string& name, const ObjectPtr<Object>& sptr_to_self) override {


If we are going with this approach, I feel we should limit this to just to the lookup ladder of functions.
i.e., its better to implement the lambda functions as separate functions

Moreover, since this is main interface to runtime.Module, I think we should provide documentation of the functions and arguments -- maybe once the functions are seperated out, it could be the documentation of those functions.

i agree with that--however, we do need the lambda function to capture sptr_to_self (this mimics the Python descriptor get() implementation). i moved the body into a separate function to align this class for a future world where we implemented the auto-generated interface.

cc @jroesch who has a prototype of the auto-generator

…tvm-build

areusch · 2021-06-29T00:28:29Z

@manupa-arm please let me know if there's anything else--i believe your comments are all forward-looking, but want to understand if there are specific changes needed here to merge.

areusch · 2021-06-29T00:28:50Z

@giuseros please take another look and explicitly approve if you're ok with this

manupak

Yes LGTM!.

ps : that generator would be handy :) @jroesch .

areusch · 2021-06-30T15:59:31Z

oops, think someone already took MLF version 3. added a patch to rev to v4.

@giuseros please take a look and explicitly approve if you're ok with this PR!

leandron

LGTM! Thanks @areusch!

leandron · 2021-07-02T22:11:53Z

Merged now, thanks @manupa-arm @giuseros @areusch!

* rename _update_target and document its function * make tvm.build return OperatorModule to return multiple outputs * allow retrieving the var names used in TIR repr * add Operator Model Library Format and test * Add pathlib convenience functions to utils.TempDirectory. * fix tests * black format * git-clang-format * pylint fixes * add asf header * change memory map to make more sense, fix tests * address giuseros comments * align GetVarName with future TypedPackedFunc * fix test * clang-format * rev model library format to v4 (bad merge)

areusch marked this pull request as ready for review May 20, 2021 17:04

giuseros requested changes May 25, 2021

View reviewed changes

manupak requested changes May 26, 2021

View reviewed changes

areusch added 12 commits June 21, 2021 08:03

rename _update_target and document its function

51e841c

make tvm.build return OperatorModule to return multiple outputs

76ef467

allow retrieving the var names used in TIR repr

66126f4

add Operator Model Library Format and test

010f8ff

Add pathlib convenience functions to utils.TempDirectory.

d0aa180

fix tests

408d154

black format

2537d3a

git-clang-format

c04e8f3

pylint fixes

9fff102

add asf header

b007dfa

change memory map to make more sense, fix tests

5c20bd6

address giuseros comments

7b1ef1a

areusch force-pushed the model-library-format-tvm-build branch from 2957cde to 7b1ef1a Compare June 21, 2021 15:03

manupak reviewed Jun 21, 2021

View reviewed changes

areusch added 3 commits June 28, 2021 16:46

Merge remote-tracking branch 'origin/main' into model-library-format-…

493b13d

…tvm-build

align GetVarName with future TypedPackedFunc

32abcf2

fix test

be680af

clang-format

ed3008d

manupak approved these changes Jun 30, 2021

View reviewed changes

rev model library format to v4 (bad merge)

c200ef5

areusch mentioned this pull request Jul 1, 2021

[microTVM] Project API infrastructure #8380

Merged

leandron approved these changes Jul 2, 2021

View reviewed changes

leandron merged commit 970aeff into apache:main Jul 2, 2021

junrushao mentioned this pull request Nov 1, 2021

Apache TVM v0.8 Release Note Candidate #9416

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "operator" style to Model Library Format #8072

Add "operator" style to Model Library Format #8072

areusch commented May 18, 2021 •

edited

Loading

areusch commented May 25, 2021

giuseros left a comment

giuseros May 25, 2021

areusch Jun 21, 2021

giuseros May 25, 2021

areusch Jun 21, 2021

giuseros May 25, 2021

areusch Jun 9, 2021

manupak left a comment

manupak May 26, 2021

areusch Jun 9, 2021

manupak Jun 10, 2021 •

edited

Loading

areusch Jun 17, 2021

manupak Jun 21, 2021 •

edited

Loading

manupak May 26, 2021

areusch Jun 9, 2021

manupak Jun 10, 2021

manupak May 26, 2021

areusch Jun 9, 2021 •

edited

Loading

manupak Jun 10, 2021

areusch Jun 17, 2021

manupak May 26, 2021

manupak Jun 21, 2021

areusch commented Jun 21, 2021

manupak left a comment •

edited

Loading

manupak Jun 21, 2021 •

edited

Loading

areusch Jun 29, 2021

areusch commented Jun 29, 2021

areusch commented Jun 29, 2021

manupak left a comment

areusch commented Jun 30, 2021

leandron left a comment

leandron commented Jul 2, 2021



		def _export_graph_model_library_format(
		mod: executor_factory.GraphExecutorFactoryModule, tempdir: pathlib.Path

		with open(src_dir / f"tir-{target_device_type}.txt", "w") as f:
		f.write(printer["print"](ir_mod))

		_populate_codegen_dir(mod, codegen_dir)


		ExportableModule = typing.Union[

Add "operator" style to Model Library Format #8072

Add "operator" style to Model Library Format #8072

Conversation

areusch commented May 18, 2021 • edited Loading

areusch commented May 25, 2021

giuseros left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manupak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manupak Jun 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manupak Jun 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

areusch Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

areusch commented Jun 21, 2021

manupak left a comment • edited Loading

Choose a reason for hiding this comment

manupak Jun 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

areusch commented Jun 29, 2021

areusch commented Jun 29, 2021

manupak left a comment

Choose a reason for hiding this comment

areusch commented Jun 30, 2021

leandron left a comment

Choose a reason for hiding this comment

leandron commented Jul 2, 2021

areusch commented May 18, 2021 •

edited

Loading

manupak Jun 10, 2021 •

edited

Loading

manupak Jun 21, 2021 •

edited

Loading

areusch Jun 9, 2021 •

edited

Loading

manupak left a comment •

edited

Loading

manupak Jun 21, 2021 •

edited

Loading