Skip to content
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

Add unregister() via DefaultQISKitProvider changes #584

Merged
merged 8 commits into from
Jun 27, 2018
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ The format is based on `Keep a Changelog`_.

Added
-----
- Add ``unregister()`` for removing previously registered providers (#584).
Copy link
Contributor

Choose a reason for hiding this comment

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

If changing the signatures or register() and unregister() for them to return the provider instances, consider adding that here too.


Changed
-------
Expand Down
7 changes: 5 additions & 2 deletions qiskit/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@
from ._quantumjob import QuantumJob
from ._quantumprogram import QuantumProgram
from ._result import Result
from .wrapper._wrapper import (available_backends, execute, register, get_backend, compile,
load_qasm_string, load_qasm_file)

from .wrapper._wrapper import (
available_backends, local_backends, remote_backends,
get_backend, compile, execute, register, unregister,
registered_providers, load_qasm_string, load_qasm_file)

# Import the wrapper, to make it available when doing "import qiskit".
from . import wrapper
Expand Down
2 changes: 1 addition & 1 deletion qiskit/_quantumprogram.py
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ def set_api(self, token, url, hub=None, group=None, project=None,
# TODO: the setting of self._api and self.__api_config is left for
# backwards-compatibility.
# pylint: disable=no-member
self.__api = qiskit.wrapper._wrapper._DEFAULT_PROVIDER.providers[-1]._api
self.__api = qiskit.wrapper._wrapper._DEFAULT_PROVIDER.providers['ibmq']._api
config_dict = {
'url': url,
}
Expand Down
4 changes: 2 additions & 2 deletions qiskit/wrapper/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@
"""

from ._wrapper import (available_backends, local_backends, remote_backends,
get_backend, compile, execute, register,
load_qasm_string, load_qasm_file)
get_backend, compile, execute, register, unregister,
registered_providers, load_qasm_string, load_qasm_file)
38 changes: 27 additions & 11 deletions qiskit/wrapper/_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
"""Helper module for simplified QISKit usage."""

import warnings

import qiskit._compiler
from qiskit import QISKitError
from qiskit.backends.ibmq.ibmqprovider import IBMQProvider
from qiskit.wrapper.defaultqiskitprovider import DefaultQISKitProvider
from ._circuittoolkit import circuit_from_qasm_file, circuit_from_qasm_string

Expand All @@ -22,7 +21,7 @@

def register(token, url='https://quantumexperience.ng.bluemix.net/api',
hub=None, group=None, project=None, proxies=None, verify=True,
provider_name='ibmq'):
provider_name=None):
"""
Authenticate against an online backend provider.

Expand All @@ -37,17 +36,34 @@ def register(token, url='https://quantumexperience.ng.bluemix.net/api',
proxies (dict): Proxy configuration for the API, as a dict with
'urls' and credential keys.
verify (bool): If False, ignores SSL certificates errors.
provider_name (str): the unique name for the online backend
provider (for example, 'ibmq' for the IBM Quantum Experience).
provider_name (str): the user-provided name for the registered
provider.
Raises:
QISKitError: if the provider name is not recognized.
"""
if provider_name == 'ibmq':
provider = IBMQProvider(token, url,
hub, group, project, proxies, verify)
_DEFAULT_PROVIDER.add_provider(provider)
else:
raise QISKitError('provider name %s is not recognized' % provider_name)
# Convert the credentials to a dict.
credentials = {
'token': token, 'url': url, 'hub': hub, 'group': group,
'project': project, 'proxies': proxies, 'verify': verify
}
_DEFAULT_PROVIDER.add_ibmq_provider(credentials, provider_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since add_ibmq_provider returns the instance, should register return the instance too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Related to the comment at #584 (comment) - I went for being conservative when exposing the providers to the user in the wrapper, but happy to reevaluate eventually.



def unregister(provider_name):
"""
Removes a provider of list of registered providers.

Args:
provider_name (str): The unique name for the online provider.
Raises:
QISKitError: if the provider name is not valid.
"""
_DEFAULT_PROVIDER.remove_provider(provider_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

If modifying remove_provider to return the instance being removed, remember to add a return here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I'm unsure if it would play nicely with the philosophy of not having the end-user-oriented wrapper functions expose the providers directly (and in this particular case, would return a "not longer in use provider"). It's a good point, though - perhaps we can reevaluate in the parent PR?



def registered_providers():
"""Return the names of the currently registered providers."""
return list(_DEFAULT_PROVIDER.providers.keys())


# Functions for inspecting and retrieving backends.
Expand Down
86 changes: 77 additions & 9 deletions qiskit/wrapper/defaultqiskitprovider.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@

"""Meta-provider that aggregates several providers."""
import logging

from itertools import combinations

from qiskit import QISKitError
from qiskit.backends.baseprovider import BaseProvider
from qiskit.backends.ibmq import IBMQProvider
from qiskit.backends.local.localprovider import LocalProvider

logger = logging.getLogger(__name__)
Expand All @@ -22,12 +24,12 @@ class DefaultQISKitProvider(BaseProvider):
def __init__(self):
super().__init__()

# List of providers.
self.providers = [LocalProvider()]
# Dict of providers.
self.providers = {'local': LocalProvider()}

def get_backend(self, name):
name = self.resolve_backend_name(name)
for provider in self.providers:
for provider in self.providers.values():
try:
return provider.get_backend(name)
except KeyError:
Expand All @@ -45,7 +47,7 @@ def available_backends(self, filters=None):
"""
# pylint: disable=arguments-differ
backends = []
for provider in self.providers:
for provider in self.providers.values():
backends.extend(provider.available_backends(filters))
return backends

Expand All @@ -60,7 +62,7 @@ def aliased_backend_names(self):
ValueError: if a backend is mapped to multiple aliases
"""
aliases = {}
for provider in self.providers:
for provider in self.providers.values():
aliases = {**aliases, **provider.aliased_backend_names()}
for pair in combinations(aliases.values(), r=2):
if not set.isdisjoint(set(pair[0]), set(pair[1])):
Expand All @@ -76,19 +78,85 @@ def deprecated_backend_names(self):
dict[str: list[str]]: aggregated alias dictionary
"""
deprecates = {}
for provider in self.providers:
for provider in self.providers.values():
deprecates = {**deprecates, **provider.deprecated_backend_names()}

return deprecates

def add_provider(self, provider):
def add_provider(self, provider, provider_name):
"""
Add a new provider to the list of known providers.

Args:
provider (BaseProvider): Provider instance.
provider_name (str): User-provided name for the provider.

Returns:
BaseProvider: the provider instance.

Raises:
QISKitError: if a provider with the same name is already in the
list.
"""
if provider_name in self.providers.keys():
raise QISKitError(
'A provider with name "%s" is already registered.'
% provider_name)

self.providers[provider_name] = provider
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking: should we return the provider to keep it consistent with add_ibmq_provider?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, makes sense!


return provider

def add_ibmq_provider(self, credentials_dict, provider_name=None):
"""
Add a new IBMQProvider to the list of known providers.

Args:
credentials_dict (dict): dictionary of credentials for a provider.
provider_name (str): User-provided name for the provider. A name
will automatically be assigned if possible.
Raises:
QISKitError: if a provider with the same name is already in the
list; or if a provider name could not be assigned.
Returns:
IBMQProvider: the new IBMQProvider instance.
"""
# Automatically assign a name if not specified.
if not provider_name:
if 'quantumexperience' in credentials_dict['url']:
provider_name = 'ibmq'
elif 'q-console' in credentials_dict['url']:
provider_name = 'qnet'
else:
raise QISKitError(
'Cannot parse provider name from credentials.')

if provider_name in self.providers.keys():
raise QISKitError(
'A provider with name "%s" is already registered.'
% provider_name)

ibmq_provider = IBMQProvider(**credentials_dict)
self.providers[provider_name] = ibmq_provider

return ibmq_provider

def remove_provider(self, provider_name):
"""
Remove a provider from the list of known providers.

Args:
provider_name (str): name of the provider to be removed.

Raises:
QISKitError: if the provider name is not valid.
"""
self.providers.append(provider)
if provider_name == 'local':
raise QISKitError("Cannot unregister 'local' provider.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you elaborate a bit about why you can not register nor unregister the local provider? Something like 'local' provider represents the set of local simulators and they are always present.

However, there would be no problem in unregistering the 'local' provider if you'd return the instance being removed so the user can register it again later or with another name:

local = DEFAULT_PROVIDER.remove_provider('local')
DEFAULT_PROVIDER.add_provider(my_provider, 'local')
DEFAULT_PROVIDER.add_provider(local, 'qiskit_local')

The above is optional and non-blocking.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's very much up for debate - it was mostly based on https://github.com/QISKit/qiskit-core/pull/547/files/1ca64661029eba38d25edcc5c1f0ba61388c9581#diff-c8057d22bae58d0abb388944221cc41d. I'd be happy to re-discuss it when the other PR is addressed, as I don't have a strong opinion either way!

try:
self.providers.pop(provider_name)
except KeyError:
raise QISKitError("'%s' provider is not registered.")

def resolve_backend_name(self, name):
"""Resolve backend name from a possible short alias or a deprecated name.
Expand Down
7 changes: 7 additions & 0 deletions test/python/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import unittest
from unittest.util import safe_repr
from qiskit import __path__ as qiskit_path
from qiskit.wrapper.defaultqiskitprovider import DefaultQISKitProvider


class Path(Enum):
Expand Down Expand Up @@ -57,6 +58,12 @@ def setUpClass(cls):
logging.INFO)
cls.log.setLevel(level)

def tearDown(self):
# Reset the default provider, as in practice it acts as a singleton
# due to importing the wrapper from qiskit.
from qiskit.wrapper import _wrapper
_wrapper._DEFAULT_PROVIDER = DefaultQISKitProvider()

@staticmethod
def _get_resource_path(filename, path=Path.TEST):
""" Get the absolute path to a resource.
Expand Down
33 changes: 1 addition & 32 deletions test/python/test_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@
"""Backends Test."""

import json
import unittest

import jsonschema

import qiskit.wrapper
from qiskit.backends.ibmq import IBMQProvider
from qiskit.wrapper.defaultqiskitprovider import DefaultQISKitProvider
from .common import requires_qe_access, QiskitTestCase, Path
from .common import Path, QiskitTestCase, requires_qe_access


def remove_backends_from_list(backends):
Expand Down Expand Up @@ -220,32 +218,3 @@ def test_remote_backend_parameters(self, QE_TOKEN, QE_URL,
'last_update_date',
'qubits',
'backend')))

@requires_qe_access
def test_wrapper_register_ok(self, QE_TOKEN, QE_URL,
hub=None, group=None, project=None):
"""Test wrapper.register()."""
qiskit.wrapper.register(QE_TOKEN, QE_URL, hub, group, project, provider_name='ibmq')
backends = qiskit.wrapper.available_backends()
backends = remove_backends_from_list(backends)
self.log.info(backends)
self.assertTrue(len(backends) > 0)

@requires_qe_access
def test_wrapper_available_backends_with_filter(self, QE_TOKEN, QE_URL,
hub=None, group=None, project=None):
"""Test wrapper.available_backends(filter=...)."""
qiskit.wrapper.register(QE_TOKEN, QE_URL, hub, group, project, provider_name='ibmq')
backends = qiskit.wrapper.available_backends({'local': False, 'simulator': True})
self.log.info(backends)
self.assertTrue(len(backends) > 0)

def test_wrapper_local_backends(self):
"""Test wrapper.local_backends(filter=...)."""
local_backends = qiskit.wrapper.local_backends()
self.log.info(local_backends)
self.assertTrue(len(local_backends) > 0)


if __name__ == '__main__':
unittest.main(verbosity=2)
5 changes: 2 additions & 3 deletions test/python/test_quantumprogram.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ def setUp(self):
"size": 3}]
}]
}
self.qp_program_finished = False
self.qp_program_exception = Exception()

###############################################################
# Tests to initiate an build a quantum program
Expand Down Expand Up @@ -1599,7 +1597,8 @@ def test_timeout(self):
# TODO: use the backend directly when the deprecation is completed.
from ._mockutils import DummyProvider
import qiskit.wrapper
qiskit.wrapper._wrapper._DEFAULT_PROVIDER.add_provider(DummyProvider())
qiskit.wrapper._wrapper._DEFAULT_PROVIDER.add_provider(DummyProvider(),
'dummy')

q_program = QuantumProgram(specs=self.QPS_SPECS)
qr = q_program.get_quantum_register("q_name")
Expand Down
Loading