TVMC: Add new text/relay frontend #10941

PhilippvK · 2022-04-08T17:35:30Z

This feature enables passing a textural representation of a relay module to the tvmc command line.

Example: tvmc compile relay.txt --target c --runtime=crt --executor=aot --executor-aot-unpacked-api=1 --pass-config tir.disable_vectorize=1 -f mlf

Currently it is not possible to supply parameters as it is mainly intended to be used for testing certain relay functions or operators. In the future (with minor changes to the tvmc frontend api) params could be passed via an additional i.e. params.bin file

This commit also adds minimal unit testing of the added feature.

PhilippvK · 2022-04-08T17:38:53Z

@masahi @gromero @leandron @jwfromm Please let me know if this is a meaningful addition to the TVMC command line interface. I am sure we could also agree on a way to provide actual values for the module parameters.

leandron

In general the functionality looks good. I have reservations about calling it "text" frontend. My suggestion is to refer to it as "Relay" frontend, so that it is more specific. See my suggestions below, and if you agree, adjust in the other places affected, for example, log messages.

python/tvm/driver/tvmc/frontends.py

PhilippvK · 2022-04-11T15:49:05Z

@leandron Thank you for your feedback. I already considered using the name relay instead. I just though RelayFrontend might easy be confused with the implementations in tvm.relay.frontend.*.

What do you think about adding a way to provide custom (non-constant) data for the weights? This would involve small changes to the TVMC command line as more that one input file (e.g. mod.relay and mod.params) would need to be allowed in this special case.

leandron · 2022-04-11T16:00:23Z

@leandron Thank you for your feedback. I already considered using the name relay instead. I just though RelayFrontend might easy be confused with the implementations in tvm.relay.frontend.*.

I think the package makes it clear as a distinction, so I'd be in favour or renaming TextFrontend to RelayFrontend, given it is located at tvm.driver.tvmc.frontend.RelayFrontend, as a module.

What do you think about adding a way to provide custom (non-constant) data for the weights? This would involve small changes to the TVMC command line as more that one input file (e.g. mod.relay and mod.params) would need to be allowed in this special case.

Not sure, it feels like we should think about a file format that incorporate all these for Relay, so that we bundle all that is needed in a single file to be used and reused in TVM, rather making it a special case in tvmc? it would be the "relay" export format, so to speak. cc @areusch

areusch

thanks @PhilippvK, i agree with calling it relay frontend but am very supportive of adding this!

python/tvm/driver/tvmc/frontends.py

PhilippvK · 2022-04-12T10:42:31Z

Thanks for your feedback. I renamed all instances of test to relay.

The recommended approach for bundling the relay format with the parameters/weights wouldn't be solved in this PR, right?

As recommended by @areusch I could add a warning like Parameters in the module will be initialized with ones.. Any ideas for a different message?

leandron · 2022-04-12T11:07:42Z

The recommended approach for bundling the relay format with the parameters/weights wouldn't be solved in this PR, right?

You're correct. This would need to be agreed within the TVM community in an RFC.

As recommended by @areusch I could add a warning like Parameters in the module will be initialized with ones.. Any ideas for a different message?

I think this message is OK given it is the Relay frontend and it has limited scope in terms of published models.

However, I would suggest, for the sake of having a better way to provide Relay in future (not only in tvmc, but for TVM in general), to start that discussion above (regarding a Relay format that includes parameters) in the community forum - https://discuss.tvm.apache.org - what do you think?

PhilippvK · 2022-04-13T04:28:05Z

@leandron I totally agree. I will start a thread in the discussion forum in the next few days.

leandron · 2022-04-14T15:41:17Z

This failed in CI due to what is being fixed in #11007. Once that gets merged, I think we can re-trigger CI here to get this merged.

leandron

LGTM, thanks!

PhilippvK · 2022-04-15T09:28:24Z

~~@leandron @areusch CI has successfully passed. Feel free to merge.~~

PhilippvK · 2022-04-16T18:07:16Z

I just ran into a few issues with the implementation when providing more complex models. I will have a look at but, but please do not merge this in its current state. Sorry for the inconvenience.

areusch · 2022-04-19T23:54:51Z

@PhilippvK we could also merge and you could submit some fixes. could you elaborate?

PhilippvK · 2022-04-20T13:38:36Z

@areusch

These are the two issues I ran into:

As the frontend is not able to distinguish between real model inputs and parameters, I propagated all of the relay inputs with constant values. This way the model inputs are removed due to Constant Folding which makes the resulting model less complex. Current Workaround: Use --input-shapes "x:[1,2,3]" etc. (if available) to get the names of the actual inputs and skip them when generating the parameter values.
I wanted to experiment with some models which make use of meta[relay.Constant] for internal constant vectors. These relay models can only be parsed if the Relay model was exported via ir_mod.astext() (which uses show_meta_data=True by default). The relay.txt format used in the MLF archive is using str(ir_mod) does neither include the #[metadata] section nor the #[version ...] header. I added a checker to detect if one this information is missing.

@areusch Feel free to have another look at the changes.

areusch · 2022-04-21T00:27:41Z

hm. for (1), it seems like we could do something like:

add a parameter which you have to pass for Relay models indicating the inputs
accept params.npz and treat everything not in there as inputs
a combination of both

for (2), that seems like an oversight on Model Library Format part. it seems like we should make tvmc validate the version numbers in relay models. Perhaps we should change Model Library Format?

i suppose it's also a bit of a burden on users authoring relay to figure out the presently-supported version, and they probably always just mean "the current version." so maybe allowing some implicitness there makes sense.

@leandron what are your thoughts?

This feature enables passing a textural representation of a relay module to the tvmc command line. Example: `tvmc compile relay.txt --target c --runtime=crt --executor=aot --executor-aot-unpacked-api=1 --pass-config tir.disable_vectorize=1 -f mlf` Currently it is not possible to supply parameters as it is mainly intended to be used for testing certain relay functions or operators. In the future (with minor changes to the tvmc frontend api) params could be passed via an additional i.e. `params.bin` file This commit also adds minimal unit testing of the added feature. Resolve PR comments TVMC: add warning if relay frontend is used

…t-shapes is provided This prevents that the constants inputs are used for Constant folding, thus changing the complexity of the model. If there would be a way, to distinguish between model inputs and parameter this workaround would not be required.

…m.parser.fromtext()

PhilippvK · 2022-07-15T07:27:01Z

@areusch

add a parameter which you have to pass for Relay models indicating the inputs

I currently already use --input-shapes to get the input names, ignoring the provided shapes. Adding new flag for only the names would be somehow redundant in my opinion.

accept params.npz and treat everything not in there as inputs

Would maybe be a good idea for the future, but I think this is out-of scope for this PR.

for (2), that seems like an oversight on Model Library Format part.

The thing is that exporting the full relay model including metadata can result in very lang files which might not be desirable for the relay.txt in the MLF. However I would prefer some consistency here having at least the proper header in there.

it seems like we should make tvmc validate the version numbers in relay models.

I can look into this.

leandron · 2022-07-19T15:21:09Z

@PhilippvK I'm merging this after a long time. It's a great new feature - sorry it took so long to realise it was pending.

* TVMC: Add new text/relay frontend This feature enables passing a textural representation of a relay module to the tvmc command line. Example: `tvmc compile relay.txt --target c --runtime=crt --executor=aot --executor-aot-unpacked-api=1 --pass-config tir.disable_vectorize=1 -f mlf` Currently it is not possible to supply parameters as it is mainly intended to be used for testing certain relay functions or operators. In the future (with minor changes to the tvmc frontend api) params could be passed via an additional i.e. `params.bin` file This commit also adds minimal unit testing of the added feature. Resolve PR comments TVMC: add warning if relay frontend is used * [TVMC] populate parameters with random values instead of ones * [TVMC] Relay frontend: do not populate input tensor buffers if --input-shapes is provided This prevents that the constants inputs are used for Constant folding, thus changing the complexity of the model. If there would be a way, to distinguish between model inputs and parameter this workaround would not be required. * [TVMC] Relay frontend: check provided file contents before calling tvm.parser.fromtext()

PhilippvK force-pushed the tvmc-text-frontend branch 2 times, most recently from fbeb139 to ca4962a Compare April 8, 2022 17:45

leandron requested changes Apr 11, 2022

View reviewed changes

python/tvm/driver/tvmc/frontends.py Outdated Show resolved Hide resolved

python/tvm/driver/tvmc/frontends.py Outdated Show resolved Hide resolved

areusch reviewed Apr 11, 2022

View reviewed changes

python/tvm/driver/tvmc/frontends.py Outdated Show resolved Hide resolved

PhilippvK requested a review from leandron April 13, 2022 04:26

leandron approved these changes Apr 14, 2022

View reviewed changes

PhilippvK force-pushed the tvmc-text-frontend branch from a19ae3a to 83736e2 Compare April 15, 2022 04:36

PhilippvK force-pushed the tvmc-text-frontend branch 2 times, most recently from b6328a4 to ecc49b0 Compare April 20, 2022 12:20

PhilippvK added 4 commits July 15, 2022 09:16

[TVMC] populate parameters with random values instead of ones

6a12a9e

[TVMC] Relay frontend: check provided file contents before calling tv…

2797383

…m.parser.fromtext()

PhilippvK force-pushed the tvmc-text-frontend branch from ecc49b0 to 2797383 Compare July 15, 2022 07:17

leandron merged commit 4d95f2c into apache:main Jul 19, 2022

TVMC: Add new text/relay frontend #10941

TVMC: Add new text/relay frontend #10941

Uh oh!

Conversation

PhilippvK commented Apr 8, 2022

Uh oh!

PhilippvK commented Apr 8, 2022

Uh oh!

leandron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

PhilippvK commented Apr 11, 2022

Uh oh!

leandron commented Apr 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

areusch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PhilippvK commented Apr 12, 2022

Uh oh!

leandron commented Apr 12, 2022

Uh oh!

PhilippvK commented Apr 13, 2022

Uh oh!

leandron commented Apr 14, 2022

Uh oh!

leandron left a comment

Choose a reason for hiding this comment

Uh oh!

PhilippvK commented Apr 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PhilippvK commented Apr 16, 2022

Uh oh!

areusch commented Apr 19, 2022

Uh oh!

PhilippvK commented Apr 20, 2022

Uh oh!

areusch commented Apr 21, 2022

Uh oh!

PhilippvK commented Jul 15, 2022

Uh oh!

leandron commented Jul 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

leandron commented Apr 11, 2022 •

edited

Loading

PhilippvK commented Apr 15, 2022 •

edited

Loading