-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
plugin convenience function for getting the entry point object #9275
Conversation
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 4490507190
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems reasonable, would it also make sense to return a dict of all possibilities?
You mean something like this? >>> entry_point_obj_by_stage('routing')
{'basic': <obj>, 'lookahead': <obj>, ...} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems generally useful to me for easy discoverability for me. I'd perhaps prefer to call the function something that makes it clear that it relates to pass manager stage plugins since "entry point" is just generic Python terminology and Qiskit also has entry points for synthesis plugins.
I'm fine to add the idea that Thomas suggested as well, though left to my own devices I probably wouldn't bother - as far as I understand it, it just boils down to
def passmanager_stage_plugin_map(stage):
manager = getattr(PassManagerStagePluginManager(), f"{stage}_plugins")
return {name: manager[name].obj for name in manager.names()}
which I'm not sure is worth the extra function, but I think it's a very subjective answer, so I don't feel strongly.
Co-authored-by: Jake Lishman <[email protected]>
I went for |
@@ -161,10 +161,11 @@ def pass_manager(self, pass_manager_config, optimization_level): | |||
PassManagerStagePlugin | |||
PassManagerStagePluginManager | |||
list_stage_plugins | |||
entry_point_obj |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the docs failure is just because this line needs updating with the new name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeap.. forgot that. Fixed in 6135d9f
from qiskit.transpiler.preset_passmanagers.plugin import passmanager_stage_plugins | ||
routing_plugins = passmanager_stage_plugins('routing') | ||
basic_plugin = routing_plugins['basic'] | ||
?basic_plugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ?
syntax is specific to IPython - you maybe want to use the standard Python built-in help
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 6135d9f
Symengine 0.10!! 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was proposed well ahead of the 0.24 feature freeze (and the 0.23 feature freeze!) and its test failures were related to other problems in CI at the time. I've just brought it up-to-date (I was already a co-author), and assuming its tests pass immediately, I'm happy to just fold it into the rest of the release.
…t#9275) * plugin convenience function for getting the entry point object * simpler code, thanks to Jake Co-authored-by: Jake Lishman <[email protected]> * typehint and docstring * reno update * thanks Jake * Fix docs build --------- Co-authored-by: Jake Lishman <[email protected]>
I was playing with staged pass managers when a wild plugin appears!
I never saw
ibm_dynamic_circuits
before so I was curious to understand where it was coming from. I can imagine this might happen more and more often as the Qiskit plugin ecosystem grows. So I createdentry_point_obj
to get extra information about it:In this way, you can not only know from where the plugin is coming, but also check the docstrings and get more help on what it does.