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

Adding compiled QASM to the result #688

Merged
merged 7 commits into from
Aug 4, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Removed

Fixed
-----
- Fixed ``get_ran_qasm`` methods on ``Result`` instances (#688)
- Fixed ``probabilities_ket`` computation in C++ simulator. (#580)
- Fixed bug in the definition of ``cswap`` gate and its test (#685).
- Fixed the examples to be compatible with version 0.5+ (#672)
Expand Down
24 changes: 24 additions & 0 deletions qiskit/_result.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,16 @@

"""Module for working with Results."""

import logging
import copy
import numpy
from ._qiskiterror import QISKitError
from ._quantumcircuit import QuantumCircuit


logger = logging.getLogger(__name__)


class Result(object):
""" Result Class.

Expand Down Expand Up @@ -414,3 +418,23 @@ def get_qubitpol_vs_xval(self, nqubits, xvals_dict=None):
circuit_name, z_dicts[qubit_ind])

return qubitpol, xvals


def copy_qasm_from_qobj_into_result(qobj, result):
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be a private method?

Copy link
Contributor 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 so. It is related to the Result type but it does not seem like an operation on the Result type. Ideally, it should be placed near the simulators which are the ones using it but I did not want to add a new file nor I was sure about where to add it.

"""Find the QASMs belonging to the Qobj experiments and copy them
into the corresponding result entries."""
for experiment in qobj.experiments:
name = experiment.header.name
qasm = getattr(experiment.header, 'compiled_circuit_qasm', None)
experiment_result = _find_experiment_result(result, name)
if qasm and experiment_result:
experiment_result['compiled_circuit_qasm'] = qasm


def _find_experiment_result(result, name):
Copy link
Member

Choose a reason for hiding this comment

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

Can you reeevaluate if this function is really needed? Seems we could avoid an extra function if it is made a one-liner with next(x for x in result['result'] if blah) (not really blocking anyway).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I try to avoid this kind of one-liners since they are, usually, harder to read. I can move it inside copy_qasm_from_qobj_into_result as an auxiliary function or a lambda variable, though.

Copy link
Member

Choose a reason for hiding this comment

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

I also dislike lots of small functions, but your choice.

for experiment_result in result['result']:
if experiment_result['name'] == name:
return experiment_result

logger.warning('No result found for experiment %s', name)
return None
Copy link
Member

Choose a reason for hiding this comment

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

This one I'm a bit more concerned about: is there really a case where it could really return None? If that is the case, line 427 would fail at:

experiment_result['compiled_circuit_qasm'] = qasm

Copy link
Contributor Author

@delapuente delapuente Jul 31, 2018

Choose a reason for hiding this comment

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

As far as I know, nope, it would expose an inconsistency between the Qobj and Result instances but the linter complained if I did not add a return value for this code-path. I can add a warning and avoid accessing it if None is returned. Does it make sense?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think explicitly taking care of the None case would be good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to keep this as a private function. With the one-liner, I can not add the warning and by moving it inside the copy_qasm_from_qobj_into_result function, makes the function hard to read.

3 changes: 2 additions & 1 deletion qiskit/backends/local/qasm_simulator_cpp.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import numpy as np

from qiskit._result import Result
from qiskit._result import Result, copy_qasm_from_qobj_into_result
from qiskit.backends import BaseBackend
from qiskit.backends.local.localjob import LocalJob
from qiskit.qobj import qobj_to_dict
Expand Down Expand Up @@ -78,6 +78,7 @@ def run(self, qobj):
def _run_job(self, qobj):
self._validate(qobj)
result = run(qobj, self._configuration['exe'])
copy_qasm_from_qobj_into_result(qobj, result)
return Result(result)

def _validate(self, qobj):
Expand Down
4 changes: 3 additions & 1 deletion qiskit/backends/local/qasm_simulator_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@

import numpy as np

from qiskit._result import Result
from qiskit._result import Result, copy_qasm_from_qobj_into_result
from qiskit.backends import BaseBackend
from qiskit.backends.local.localjob import LocalJob
from ._simulatorerror import SimulatorError
Expand Down Expand Up @@ -291,6 +291,8 @@ def _run_job(self, qobj):
'status': 'COMPLETED',
'success': True,
'time_taken': (end - start)}

copy_qasm_from_qobj_into_result(qobj, result)
return Result(result)

def run_circuit(self, circuit):
Expand Down
16 changes: 13 additions & 3 deletions qiskit/backends/local/unitary_simulator_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,11 @@
"""
import logging
import uuid
import time

import numpy as np

from qiskit._result import Result
from qiskit._result import Result, copy_qasm_from_qobj_into_result
from qiskit.backends import BaseBackend
from qiskit.backends.local.localjob import LocalJob
from ._simulatortools import enlarge_single_opt, enlarge_two_opt, single_gate_matrix
Expand Down Expand Up @@ -161,11 +162,20 @@ def _run_job(self, qobj):
Result: Result object
"""
result_list = []
start = time.time()
for circuit in qobj.experiments:
result_list.append(self.run_circuit(circuit))
end = time.time()
job_id = str(uuid.uuid4())
return Result(
{'job_id': job_id, 'result': result_list, 'status': 'COMPLETED'})
result = {'backend': self._configuration['name'],
'id': qobj.qobj_id,
'job_id': job_id,
'result': result_list,
'status': 'COMPLETED',
'sucsess': True,
'time_taken': (end - start)}
copy_qasm_from_qobj_into_result(qobj, result)
return Result(result)

def run_circuit(self, circuit):
"""Apply the single-qubit gate.
Expand Down
34 changes: 33 additions & 1 deletion test/python/test_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,24 @@

import qiskit.wrapper
from qiskit import QISKitError
from qiskit import QuantumRegister, ClassicalRegister, QuantumCircuit
from qiskit.backends.ibmq import IBMQProvider
from qiskit.wrapper import registered_providers
from qiskit.wrapper import registered_providers, execute
from ._mockutils import DummyProvider
from .common import QiskitTestCase, requires_qe_access
from .common import is_cpp_simulator_available
from .test_backends import remove_backends_from_list


class TestWrapper(QiskitTestCase):
"""Wrapper test case."""
def setUp(self):
q = QuantumRegister(3)
c = ClassicalRegister(3)
self.circuit = QuantumCircuit(q, c)
self.circuit.ccx(q[0], q[1], q[2])
self.circuit.measure(q, c)

@requires_qe_access
def test_wrapper_register_ok(self, QE_TOKEN, QE_URL, hub, group, project):
"""Test wrapper.register()."""
Expand Down Expand Up @@ -110,6 +119,29 @@ class SecondDummyProvider(DummyProvider):
self.assertEqual(dummy_backend,
qiskit.wrapper.get_backend('local_dummy_simulator'))

def test_local_execute_and_get_ran_qasm(self):
"""Check if the local backend return the ran qasm."""

cpp_simulators = [
'local_qasm_simulator_cpp',
'local_statevector_simulator_cpp'
]

python_simulators = [
'local_qasm_simulator_py',
'local_statevector_simulator_py',
'local_unitary_simulator_py'
]

local_simulators = python_simulators
if is_cpp_simulator_available():
local_simulators += cpp_simulators

for backend_name in local_simulators:
with self.subTest(backend_name=backend_name):
result = execute(self.circuit, 'local_qasm_simulator').result()
self.assertIsNotNone(result.get_ran_qasm(self.circuit.name))


if __name__ == '__main__':
unittest.main(verbosity=2)