Skip to content

[microTVM][ARM] Add Relay tests for conv2d registered schedules#11250

Merged
mehrdadh merged 14 commits into
apache:mainfrom
mehrdadh:microtvm/add_conv2d_tests
May 19, 2022
Merged

[microTVM][ARM] Add Relay tests for conv2d registered schedules#11250
mehrdadh merged 14 commits into
apache:mainfrom
mehrdadh:microtvm/add_conv2d_tests

Conversation

@mehrdadh

@mehrdadh mehrdadh commented May 9, 2022

Copy link
Copy Markdown
Member

This PR does the following changes:

  1. Adds a dispatcher where we can force a relay module to build by a specific schedule name
  2. Adds relay level test for various conv2d schedules
  3. Moves aot test utils to python package to be accessible from python package.

cc @gromero

@mehrdadh

mehrdadh commented May 9, 2022

Copy link
Copy Markdown
Member Author

cc @mkatanbaf

@mehrdadh mehrdadh force-pushed the microtvm/add_conv2d_tests branch 4 times, most recently from 3d6106d to 0a19a88 Compare May 11, 2022 23:30
@mehrdadh mehrdadh marked this pull request as ready for review May 11, 2022 23:31
@mehrdadh

Copy link
Copy Markdown
Member Author

cc @areusch

@mehrdadh mehrdadh requested a review from areusch May 12, 2022 17:34

@areusch areusch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks @mehrdadh ! i think this is a good utility that can be used to run integration tests. thinking about it a bit more, i think that we may want to complement this with a faster unit test infrastructure, but that we could proceed with it for writing micro tests since these need the tvm.relay.build() output to work with Project API for now.

cc @tqchen @junrushao1994 if they could comment more on the intended way that someone could verify in test that a given schedule was part of the operator strategy for a particular Target.

Comment thread python/tvm/autotvm/task/dispatcher.py Outdated
Name of schedule to use.
"""

def __init__(self, tasks, schedule_name: str):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

for schedule_name, should we be taking a mapping here, e.g. a dict mapping compute function name to schedule or something?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

can you explain why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think this would get called for each function being scheduled. so if the Relay program was more complex (e.g. task extraction found multiple tasks), then we'd want to specify a schedule for each.

Comment thread python/tvm/autotvm/task/dispatcher.py Outdated
Comment thread python/tvm/topi/arm_cpu/conv2d_int8.py
Comment thread python/tvm/micro/testing/aot_test_utils.py Outdated
Comment thread tests/python/relay/strategy/arm_cpu/test_conv2d_nhwc.py Outdated
compiled_mods.append(
AOTCompiledTestModel(model=model, executor_factory=executor_factory)
)
if schedule_name:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could we add some kind of assert here to verify that tvm.relay.build actually chose this schedule?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think there's a way to do that from AOTExecutorFactoryModule

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i was thinking we could do something like search the output function names here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We can iterate on this in a follow up PR. thanks!



class ApplyFixedConfig(DispatchContext):
"""Apply a config of a deterministic schedule.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could you explain why this is different from ApplyConfig here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

added more details.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think the comment no longer makes sense now that we are accepting an array of schedule names, but can fix that in a follow-on.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

will fix that in a follow up PR.

@ekalda ekalda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very cool!

Comment thread python/tvm/autotvm/task/dispatcher.py Outdated
Comment thread python/tvm/micro/testing/aot_test_utils.py Outdated
@github-actions github-actions Bot requested a review from gromero May 16, 2022 21:39
@mehrdadh mehrdadh force-pushed the microtvm/add_conv2d_tests branch from 152c1aa to 76c44fd Compare May 16, 2022 21:44
@mehrdadh mehrdadh requested a review from areusch May 16, 2022 22:53
@mehrdadh

Copy link
Copy Markdown
Member Author

@areusch PTAL, thanks

@areusch

areusch commented May 17, 2022

Copy link
Copy Markdown
Contributor

@mehrdadh just replied on the one thread, lmk if that makes sense



class ApplyFixedConfig(DispatchContext):
"""Apply a config of a deterministic schedule.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think the comment no longer makes sense now that we are accepting an array of schedule names, but can fix that in a follow-on.

@mehrdadh mehrdadh merged commit 5e29ddd into apache:main May 19, 2022
@mehrdadh mehrdadh deleted the microtvm/add_conv2d_tests branch May 19, 2022 23:09
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