Skip to content

Conversation

@leandron
Copy link
Contributor

@leandron leandron commented Jun 8, 2021

Expose list of PassContext configurations to the Python APIs:

  • Expose C++ PassContext::ListConfigNames() via its Python counterpart tvm.ir.transform.PassContext.list_configs()
  • Add unit tests for the C++ and Python layers
  • In a follow-up PR we'll use this API to set configurations using tvmc

cc @Mousius @gromero @manupa-arm @areusch for reviews

@junrushao
Copy link
Member

CC: @zhiics @comaniac @kevinthesun

Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

LGTM. A nit about the API name. ListAllConfigs seems a bit confusing, as it is actually listing the config names without their values. Maybe ListConfigNames might be better.

@leandron
Copy link
Contributor Author

leandron commented Jun 8, 2021

LGTM. A nit about the API name. ListAllConfigs seems a bit confusing, as it is actually listing the config names without their values. Maybe ListConfigNames might be better.

Makes sense, thanks @comaniac I've just renamed it.

@leandron
Copy link
Contributor Author

leandron commented Jun 9, 2021

cc @mbaret can you also review and possibly merge?

Copy link
Contributor

@mbaret mbaret left a comment

Choose a reason for hiding this comment

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

LGTM

 * Expose C++ PassContext::ListAllConfigs via its Python counterpart
   tvm.ir.transform.PassContext.list_configs()

 * Add unit tests for the C++ and Python layers
Copy link
Contributor

@zackcquic zackcquic left a comment

Choose a reason for hiding this comment

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

Hi @leandron:

LGTM.

May I know what kind of features (next PR as you mentioned before) depend on this change?

@leandron
Copy link
Contributor Author

leandron commented Jun 9, 2021

Hi @leandron:

LGTM.

May I know what kind of features (next PR as you mentioned before) depend on this change?

Hi @zackcquic yes, sure. I'm working on a PR to give tvmc the ability to set some PassContext configurations (the ones that can accept simple values) via the command line.

I'm actually going to update this PR to also be able to pass the types of the PassContext configurations, so that I can present better messages.

I have the tvmc changes already done, just thought that separate PRs for the changes was more appropriate, that's is why the tvmc changes are not added here.

@comaniac comaniac merged commit 1f2ca06 into apache:main Jun 9, 2021
@comaniac
Copy link
Contributor

comaniac commented Jun 9, 2021

Thanks @leandron @mbaret @zhiics @zackcquic

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jun 17, 2021
…8212)

* Expose C++ PassContext::ListAllConfigs via its Python counterpart
   tvm.ir.transform.PassContext.list_configs()

 * Add unit tests for the C++ and Python layers
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jun 17, 2021
…8212)

* Expose C++ PassContext::ListAllConfigs via its Python counterpart
   tvm.ir.transform.PassContext.list_configs()

 * Add unit tests for the C++ and Python layers
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.

6 participants