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

QuantumCircuit to PennyLane template conversion #55

Merged
merged 41 commits into from
Nov 11, 2019

Conversation

antalszava
Copy link
Contributor

Context:
Each quantum software framework has its architecture when it comes to how they handle creating circuits and applying gates. Therefore a converter between Qiskit's QuantumCircuit and a custom template in PennyLane could be provided. This converter could then be utilized by users who are new to PennyLane but have prior experience with other quantum computing software frameworks.

Description of the Change:

Adds the converter function load in converter.py that creates a PennyLane template from an arbitrary

  • QuantumCircuit object;
  • QASM file;
  • QASM string.

Adds the entrypoint pennylane.io such that PennyLane can use the load function.

Benefits:

Allows the conversion from Qiskit circuit object to Pennylane templates. Once the conversion has been done, these templates can then be placed within a quantum circuit in PennyLane.

Circuits containing parameters converted this way can be parametrized by passing a dictionary as an argument to load.

Each converted circuit can be added to a quantum circuit in PennyLane an arbitrary many times. The user can also specify different parameters each time if desired.

Further allows shifting the converted circuit. Passing the wire_shift argument allows to shift the indices of wires already defined in the original QuantumCircuit object e.g. if the QuantumCircuit acts on wires [0, 1] and if wire_shift == 1, then the returned Pennylane template will act on wires [1, 2].

Possible Drawbacks:
N/A

Related GitHub Issues:
N/A

@codecov
Copy link

codecov bot commented Oct 31, 2019

Codecov Report

Merging #55 into master will decrease coverage by 0.81%.
The diff coverage is 94.23%.

@@            Coverage Diff             @@
##           master      #55      +/-   ##
==========================================
- Coverage     100%   99.18%   -0.82%     
==========================================
  Files           8        9       +1     
  Lines         316      368      +52     
==========================================
+ Hits          316      365      +49     
- Misses          0        3       +3
Impacted Files Coverage Δ
pennylane_qiskit/qiskit_device.py 100% <ø> (ø) ⬆️
pennylane_qiskit/__init__.py 100% <100%> (ø) ⬆️
pennylane_qiskit/ops.py 100% <100%> (ø) ⬆️
pennylane_qiskit/converter.py 94% <94%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4aee6c9...ca97645. Read the comment docs.

@antalszava antalszava changed the title Quanctum circuit to template QuantumCircuit to PennyLane template conversion Oct 31, 2019
@antalszava antalszava changed the title QuantumCircuit to PennyLane template conversion QuantumCircuit to PennyLane template conversion Oct 31, 2019
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Looks great Antal, mostly minor comments. Great job with the really thorough tests!

CHANGELOG.md Outdated Show resolved Hide resolved
pennylane_qiskit/__init__.py Outdated Show resolved Hide resolved
pennylane_qiskit/converter.py Outdated Show resolved Hide resolved
pennylane_qiskit/converter.py Outdated Show resolved Hide resolved
pennylane_qiskit/converter.py Outdated Show resolved Hide resolved
tests/test_converter.py Outdated Show resolved Hide resolved
tests/test_converter.py Outdated Show resolved Hide resolved
tests/test_converter.py Outdated Show resolved Hide resolved
tests/test_converter.py Show resolved Hide resolved
tests/test_converter.py Outdated Show resolved Hide resolved
@josh146
Copy link
Member

josh146 commented Oct 31, 2019

By the way, it looks like the tests are failing on Python 3.5 - could be a dictionary ordering issue

@codecov-io
Copy link

codecov-io commented Oct 31, 2019

Codecov Report

Merging #55 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #55   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           8      9    +1     
  Lines         316    378   +62     
=====================================
+ Hits          316    378   +62
Impacted Files Coverage Δ
pennylane_qiskit/qiskit_device.py 100% <ø> (ø) ⬆️
pennylane_qiskit/__init__.py 100% <100%> (ø) ⬆️
pennylane_qiskit/ops.py 100% <100%> (ø) ⬆️
pennylane_qiskit/converter.py 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4aee6c9...4fbd76e. Read the comment docs.

@josh146
Copy link
Member

josh146 commented Nov 3, 2019

By the way @antalszava, it looks like Codecov is failing because the lines 130-131 aren't being covered by the tests:

else:
    float_params = [float(param) for param in parameters]
    operation(*float_params, wires=operation_wires)

@antalszava
Copy link
Contributor Author

Thank you for all the feedback, it was really valuable!

  1. As far as the changes in PennyLane are concerned, I would envision something like the following in __init__.py (similar to how the device entrypoint is treated):
def load(name: str, quantum_circuit_object):
    """load(name, quantum_circuit_object)
    Load a plugin :func:`~.load` which can then be used to convert objects of
    quantum circuits from other frameworks.

    Args:
        name (str): the name of the plugin to convert from
        quantum_circuit_object: the quantum circuit that will be converted
            to a PennyLane template

    Returns:
        _function: the PennyLane template created from the quantum circuit
            object
    """

    if name in plugin_converters:

        # loads the plugin load function
        plugin_converter = plugin_converters[name].load()

        # calls the load function of the converter on the quantum circuit object
        return plugin_converter(quantum_circuit_object)

    raise ValueError(
        "Converter does not exist. Make sure the required plugin is installed "
        "and supports conversion."
    )

Once this is defined, the following from_qasm can also be added

def from_qasm(quantum_circuit: str):
    """load(name, quantum_circuit)
    Loads quantum circuits defined in QASM by using the converter in the
    PennyLane-Qiskit plugin.

    Args:
        quantum_circuit (str): the name for the QASM file or string

    Returns:
        _function: the PennyLane template created from the quantum circuit
            object
    """
    return load('qasm', quantum_circuit)

In the latest version, the entrypoints were extended with:
'qasm = pennylane_qiskit:load_qasm',

Therefore, each plugin will be available through entrypoints and by passing their string as the name argument to load.

In order to make this seamless, I have created one single function for QASM and added a simple check.

Furthermore, the load function has been refactored, it has been split into smaller functions such that it is more readable.

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Really good @antalszava! My only comments were with respect to the load function docstring, and the load_qasm file. I think it's always best to have an explicit function to load from files, as it can avoid unexpected behaviour (cf. json.load for files, json.loads for strings).

Happy to approve conditional on those small remaining comments.

pennylane_qiskit/converter.py Outdated Show resolved Hide resolved
pennylane_qiskit/converter.py Outdated Show resolved Hide resolved
pennylane_qiskit/converter.py Outdated Show resolved Hide resolved
pennylane_qiskit/converter.py Outdated Show resolved Hide resolved
pennylane_qiskit/converter.py Outdated Show resolved Hide resolved
Copy link
Member

@co9olguy co9olguy left a comment

Choose a reason for hiding this comment

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

Thanks @antalszava, looking good. I left only a couple very minor comments/questions/suggestions.

tests/conftest.py Outdated Show resolved Hide resolved
pennylane_qiskit/ops.py Show resolved Hide resolved
pennylane_qiskit/converter.py Outdated Show resolved Hide resolved
tests/test_converter.py Outdated Show resolved Hide resolved
tests/test_converter.py Outdated Show resolved Hide resolved
tests/test_converter.py Outdated Show resolved Hide resolved
tests/test_converter.py Outdated Show resolved Hide resolved
pennylane_qiskit/converter.py Outdated Show resolved Hide resolved
pennylane_qiskit/converter.py Show resolved Hide resolved
@co9olguy co9olguy merged commit d852e35 into master Nov 11, 2019
@co9olguy co9olguy deleted the quanctum_circuit_to_template branch November 11, 2019 21:46
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