Skip to content

Conversation

mtreinish
Copy link
Member

Summary

This commit adds a simple and standardized interface for external
plugins to register providers and have them work natively off of the
qiskit namespace. With this anyone can setup providers to be
discoverable and automatically loaded into qiskit-terra if they're
installed. It leverages setuptools entry points to define an entry point
to basically advertise to terra that a python package provides a
backend provider. Terra now leverages the stevedore library (which just
provides a useful API ontop of entrypoints and plugins) to discover
the providers installed.

Details and comments

@mtreinish mtreinish changed the title [WIP] Add plugin interface for external provider registration Add plugin interface for external provider registration Dec 11, 2018
@ajavadia ajavadia added the on hold Can not fix yet label Dec 18, 2018
This commit adds a simple and standardized interface for external
plugins to register providers and have them work natively off of the
qiskit namespace. With this anyone can setup providers to be
discoverable and automatically loaded into qiskit-terra if they're
installed. It leverages setuptools entry points to define an entry point
to basically advertise to terra that a python package provides a
backend provider. Terra now leverages the stevedore library (which just
provides a useful API ontop of entrypoints and plugins) to discover
the providers installed.
@mtreinish mtreinish force-pushed the wip-provider-plugins branch from 86adccb to ef062e4 Compare February 6, 2019 15:37
@mtreinish mtreinish removed the on hold Can not fix yet label Feb 6, 2019
@mtreinish mtreinish requested a review from kdk as a code owner February 6, 2019 21:30
@ewinston
Copy link
Contributor

How does stevedore support prospective qiskit plugins that can't be done with builtin methods, e.g. setuptools entry_points, which it is built upon?

@mtreinish
Copy link
Member Author

The stevedore bit is just for discovery of plugins that advertise themselves as such using entry_points. If a plugin doesn't expose the entry point at install time stevedore won't do anything with it (since it won't see it). If a plugin can't use entry_points for some reasons it'll still work like things already do today where you have to manually import the plugin from wherever and access it directly (like in the case of JKU you would use from qiskit_jku_provider import JKUProvider still if it couldn't use entrypoints). This patch is only about simplifying the discovery of provider plugins and standardizing how users can interact with them, it doesn't really change anything in the plugins themselves.

@ewinston
Copy link
Contributor

My understanding was that setuptools already does discovery of plugins,
https://packaging.python.org/guides/creating-and-discovering-plugins/#using-package-metadata?

@mtreinish
Copy link
Member Author

@ewinston yeah, stevedore is just using that under the covers. It provides a nicer api to work with than the raw setuptools.

@ajavadia
Copy link
Member

So with this patch, the only difference would be that the user writes:
from qiskit import JKUProvider
instead of
from jku_provider import JKUProvider?

Not sure if that's desired. Wouldn't it somehow imply that JKUProvider belongs to qiskit, rather than being clearly a 3rd party installed package?
Also it really relies on that package doing this thing in its setup. So we may end up with an inconsistent situation where some providers are in the qiskit namespace and some are not, which we won't be able to control

@mtreinish
Copy link
Member Author

@ajavadia Well it's actually from qiskit.providers import JKUProvider, the original patch I had did it from qiskit import JKUProvider but the intent there was for Aer and IBMQ to use this which we don't need anymore. I switched it to try and outline that exact difference of this being a 3rd party provider, not an in-repo thing. So the 1st party providers that we support (Aer, BasicAer, and IBMQ) are all accessed off of qiskit.* and then the 3rd party ones which we don't support are all accessed off of qiskit.providers.*. I can easily change that path to be something else to try and highlight that difference more though, it's very straightforward.

As for some providers using vs not, it's true we can only provide the interface but not force an external provider to use. But, I figure if we clearly document it and the benefits of using it people will start to adopt it, but we can't force anyone to use it. Just like we can't really force anyone to inherit from our abstract provider classes today.

@ajavadia
Copy link
Member

So if the 3rd party provider just does a directory reorganization and puts their code under
qiskit/providers/third_party (like Aer does at the moment), would that solve the problem?

Copy link
Member

@jaygambetta jaygambetta left a comment

Choose a reason for hiding this comment

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

the doc/providers should go to qiskit/doc

@mtreinish
Copy link
Member Author

Yeah, I wrote this before the docs migration. I left it in the PR because it helps explain what the patch does. I will remove it from this PR an open up a docs one when we're ready to merge this.

@ajavadia
Copy link
Member

ajavadia commented Mar 9, 2019

@mtreinish can you address my comment above? I'm not sure that what a backend developer has to do here is different from just organizing their namespace under qiskit/providers, like Aer does.

@mtreinish
Copy link
Member Author

mtreinish commented Mar 14, 2019

@mtreinish can you address my comment above? I'm not sure that what a backend developer has to do here is different from just organizing their namespace under qiskit/providers, like Aer does.

This actually isn't a solution for third party providers. Having external provider isn't something I would recommend generally, mostly because it is confusing to have an import from qiskit that isn't maintained by a qiskit project. There are also edge cases with namespace packages which we can workaround by maintaining the projects by the same people. I'm not really comfortable encouraging external projects using that mechanism.

But even ignoring all of that, simply moving code to qiskit/providers isn't actually a solution. The way providers are used it's a global instance of the provider class. So when you call qiskit.Aer it's actually an object not the class and we need to instantiate it. For example for, Aer we do this here: https://github.com/Qiskit/qiskit-aer/blob/master/qiskit/providers/aer/__init__.py#L28 and add it to Terra here: https://github.com/Qiskit/qiskit-terra/blob/master/qiskit/__init__.py#L45-L48

This plugin provider makes the namespaces for external code live outside of qiskit.* and it provides a common interface for plugin authors to advertise that they have a provider for qiskit users. Then users have a single place they can access those instantiated Provider objects. Right now it's off of qiskit.providers but we can move that somewhere else, like qiskit.providers.plugins or qiskit.providers.external.

If you'd like I can write an example patch for JKU or another provider, or I can just write a fake provider plugin example to show how this works.

mtreinish added a commit to mtreinish/qiskit that referenced this pull request Apr 2, 2019
Terra change Qiskit/qiskit#1465 adds a plugin interface to allow
external providers to have a common interface with terra. It also
providers users a consistent place to access external providers. This
commit adds the documentation for this new feature.
@mtreinish
Copy link
Member Author

I moved the docs from here to the qiskit repo in: #190

@ajavadia ajavadia added on hold Can not fix yet and removed on hold Can not fix yet labels Apr 3, 2019
@ajavadia ajavadia closed this Apr 3, 2019
@ajavadia
Copy link
Member

ajavadia commented Apr 3, 2019

closing this until there's a clear use case for doing this (i.e. there is complaint about 3rd party provider being hard to use under its own namespace). I think there is value in the qisksit namespace being preserved for things we provide.

mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Jul 21, 2020
Namespace packages are constant source of problems for users. The python
packaging ecosystem around splitting packages across namespaces is
fragile at the best of times and can often leave a you with an
environment that isn't recoverable (especially when mixing install
methods). There is also a performance hit whenever there is a piece of
the namespace we allow external packages to extend since it requires
doing a full python path search which can be slow depending on the
backing I/O and the number of paths in sys.path for an environment. This
commit starts the process of addressing this by removing the arbitrary
namespace hook points and hard coding the element namespace maps via a
custom import loader at the root of the namespace. This has 2 advantages
it removes the use of namespace packages so the fragility and
performance impact are fixed since every element will be renamed to use
'qiskit_*' instead of 'qiskit.*', but it also makes it explicit where we
extend the namespace. The previous method allowed any package to extend
qiskit.* and qiskit.providers.* with whatever they wanted.

We'll need to coordinate updating the elements with this merging,
because it is a breaking change for each element (although not for end
users).

A potential follow on is to add a plugin interface for 3rd party
providers like what was proposed in Qiskit#1465 so that we can make external
providers externally discoverable without needing to add manual hook
points moving forward (this was done for backwards compat with the aqt
and honeywell provider).

Fixes Qiskit#559
mtreinish added a commit that referenced this pull request Aug 31, 2020
* Remove namespace packaging and hardcode elements

Namespace packages are constant source of problems for users. The python
packaging ecosystem around splitting packages across namespaces is
fragile at the best of times and can often leave a you with an
environment that isn't recoverable (especially when mixing install
methods). There is also a performance hit whenever there is a piece of
the namespace we allow external packages to extend since it requires
doing a full python path search which can be slow depending on the
backing I/O and the number of paths in sys.path for an environment. This
commit starts the process of addressing this by removing the arbitrary
namespace hook points and hard coding the element namespace maps via a
custom import loader at the root of the namespace. This has 2 advantages
it removes the use of namespace packages so the fragility and
performance impact are fixed since every element will be renamed to use
'qiskit_*' instead of 'qiskit.*', but it also makes it explicit where we
extend the namespace. The previous method allowed any package to extend
qiskit.* and qiskit.providers.* with whatever they wanted.

We'll need to coordinate updating the elements with this merging,
because it is a breaking change for each element (although not for end
users).

A potential follow on is to add a plugin interface for 3rd party
providers like what was proposed in #1465 so that we can make external
providers externally discoverable without needing to add manual hook
points moving forward (this was done for backwards compat with the aqt
and honeywell provider).

Fixes #559

* Fix lint

* Fix lint

* Handle missing ibmq in job watcher

* Suppress packaging warnings in docs job

* Use find_spec instead of find_module

* Fix lint and remove deprecated module_repr method

* Add back module_repr, pylint wants it there because of abc
mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Sep 18, 2020
Namespace packages are constant source of problems for users. The python
packaging ecosystem around splitting packages across namespaces is
fragile at the best of times and can often leave a you with an
environment that isn't recoverable (especially when mixing install
methods). There is also a performance hit whenever there is a piece of
the namespace we allow external packages to extend since it requires
doing a full python path search which can be slow depending on the
backing I/O and the number of paths in sys.path for an environment. This
commit starts the process of addressing this by removing the arbitrary
namespace hook points and hard coding the element namespace maps via a
custom import loader at the root of the namespace. This has 2 advantages
it removes the use of namespace packages so the fragility and
performance impact are fixed since every element will be renamed to use
'qiskit_' instead of 'qiskit.', but it also makes it explicit where we
extend the namespace. The previous method allowed any package to extend
qiskit.* and qiskit.providers.* with whatever they wanted.

We'll need to coordinate updating the elements with this merging,
because it is a breaking change for each element (although not for end
users).

A potential follow on is to add a plugin interface for 3rd party
providers like what was proposed in Qiskit#1465 so that we can make external
providers externally discoverable without needing to add manual hook
points moving forward (this was done for backwards compat with the aqt
and honeywell provider).

This is a second attempt at removing namespace packaging. The first
attempt in PR Qiskit#4767 was merged and had to be reverted because there were
some circular import error issues that needed to be resolved. Since
having this in terra blocks CI for all the qiskit elements a revert was
necessary to unblock developement for the entire project while those
were resolved.
mergify bot added a commit that referenced this pull request Jun 22, 2022
* Remove namespace packaging and hardcode elements (attempt 2)

Namespace packages are constant source of problems for users. The python
packaging ecosystem around splitting packages across namespaces is
fragile at the best of times and can often leave a you with an
environment that isn't recoverable (especially when mixing install
methods). There is also a performance hit whenever there is a piece of
the namespace we allow external packages to extend since it requires
doing a full python path search which can be slow depending on the
backing I/O and the number of paths in sys.path for an environment. This
commit starts the process of addressing this by removing the arbitrary
namespace hook points and hard coding the element namespace maps via a
custom import loader at the root of the namespace. This has 2 advantages
it removes the use of namespace packages so the fragility and
performance impact are fixed since every element will be renamed to use
'qiskit_' instead of 'qiskit.', but it also makes it explicit where we
extend the namespace. The previous method allowed any package to extend
qiskit.* and qiskit.providers.* with whatever they wanted.

We'll need to coordinate updating the elements with this merging,
because it is a breaking change for each element (although not for end
users).

A potential follow on is to add a plugin interface for 3rd party
providers like what was proposed in #1465 so that we can make external
providers externally discoverable without needing to add manual hook
points moving forward (this was done for backwards compat with the aqt
and honeywell provider).

This is a second attempt at removing namespace packaging. The first
attempt in PR #4767 was merged and had to be reverted because there were
some circular import error issues that needed to be resolved. Since
having this in terra blocks CI for all the qiskit elements a revert was
necessary to unblock developement for the entire project while those
were resolved.

* Try using new aer path for qiskit.Aer alias

* Try moving Aer and IBMQ alias to the end

* Fix typo

* Fix circular import issue

* Fix typos

* Run black

* Remove file encoding

* Adjust init with import re-direct

* Make qiskit_aer a opportunistic load

To ensure that someone with an old version of Aer installed can still
access it via the old namespace this changes the meta finder logic to
first try qiskit_aer, if it's importable then we build the redirect path
first and use that for the legacy qiskit.providers.aer path. If it's not
then we just return None and fall back to the other finders in
sys.meta_path.

To support this the pkgutil hook is added back to qiskit.providers to
add the namespace hook for old version of aer using namespace packagingm
although not strictly necessary because the implicit support for
namespace packages will still likely work we can remove it at a later
date.

* Deprecate qiskit.Aer entrypoint

* Fix lint

* Remove unnecessary noqa comments

* Revert version.py change and use old aer import for now

* Fix typo

* Add back pkgutil extension to root init

* Make redirect hook more generic

* Add comments explaining the various hooks

* Make qiskit.Aer a pending deprecation instead of a deprecation

* Restrict namespace redirect error to ModuleNotFoundError

* Apply suggestions from code review

Co-authored-by: Jake Lishman <jake@binhbar.com>

Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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.

4 participants