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

Rename multiplexer_dg with multiplexer_decomp_dg after UC.inverse() #8454

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hhorii
Copy link
Contributor

@hhorii hhorii commented Aug 4, 2022

Summary

#8231 produced regression in transpilation from inverse isometry to multiplexer. Isometry.inverse() generates a multiplexer gate with basic Gate class. However, originally, multiplexer is based on UCGate that has parameters.

This PR provides a work around by naming multiplexer_decomp(_dg) instead of multiplexer(_dg) to generated multiplexer gates of Gate class.

Details and comments

This bug exists before #8231. UCGate.inverse().inverse() returns Gate instance that has not any parameters even though its name is multiplexer. Isometry.inverse() internally call UCGate.inverse().inverse() since #8231.

import numpy as np
from qiskit import QuantumCircuit
from qiskit.quantum_info.states import Statevector

qc = QuantumCircuit(1,1)
vector = [0.2, 0.1]
vector_circuit = QuantumCircuit(1)
vector_circuit.isometry(vector / np.linalg.norm(vector), [0], None)
vector_circuit = vector_circuit.inverse()
qc.append(vector_circuit, [0])

sv = Statevector(qc)
gate_list = [np.array([[sv[0], -sv[1]], [sv[1], sv[0]]])]
qc = QuantumCircuit(1,1)
qc.uc(gate_list, [], [0])

uc = qc.data[0][0]
uc_inverse = uc.inverse()
uc_inverse_inverse = uc_inverse.inverse()
print(uc.__class__, ", param_len=", len(uc.params), ", name=", uc.name)
print(uc_inverse.__class__, ", param_len=", len(uc_inverse.params), ", name=", uc_inverse.name)
print(uc_inverse_inverse.__class__, ", param_len=", len(uc_inverse_inverse.params), ", name=", uc_inverse_inverse.name)

Output:

<class 'qiskit.extensions.quantum_initializer.uc.UCGate'> , param_len= 1 , name= multiplexer
<class 'qiskit.circuit.gate.Gate'> , param_len= 0 , name= multiplexer_dg
<class 'qiskit.circuit.gate.Gate'> , param_len= 0 , name= multiplexer

If multiplexer must have parameter gatelist , UCGate.inverse() should not name multiplexer_dg to a returned Gate instance because it may produce multiplexer gate with its inverse().

This PR names multiplexer_decomp_dg to a returned Gate instance produced by UCGate.inverse().

@hhorii hhorii requested a review from a team as a code owner August 4, 2022 15:40
@qiskit-bot
Copy link
Collaborator

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:

  • @Qiskit/terra-core

@mtreinish mtreinish added priority: high stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog labels Aug 4, 2022
@mtreinish mtreinish added this to the 0.21.2 milestone Aug 4, 2022
@hhorii hhorii force-pushed the revert_isometry_inverse branch 2 times, most recently from 9364fe5 to 133dd41 Compare August 5, 2022 02:01
@hhorii hhorii changed the title revert Isometry.inverse() change in #8231 Rename multiplexer_dg with multiplexer_decomp_dg after UC.inverse() Aug 8, 2022
@hhorii hhorii force-pushed the revert_isometry_inverse branch from c3a164a to 8245151 Compare August 8, 2022 03:20
@coveralls
Copy link

Pull Request Test Coverage Report for Build 2815151750

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.003%) to 84.037%

Totals Coverage Status
Change from base Build 2800102724: 0.003%
Covered Lines: 56089
Relevant Lines: 66743

💛 - Coveralls

@Cryoris
Copy link
Contributor

Cryoris commented Aug 10, 2022

I'm not sure I remember the outcome of the last meeting correctly, but did we decide that it would be nicer if UCGate.inverse() would return a UCGate again? Maybe we could implement that and and still re-use the synthesis doing something like

class UCGate(Gate):
    def inverse(self):
        definition = # compute inverse like right now
        
        # inverted gate list
        gate_list = list(reversed([np.conj(unitary.T) for unitary in self.params]))
        
        # ensure inverting twice gives again "multiplexer"
        name = "multiplexer_dg" if self.name == "multiplexer" else "multiplexer"
        
        inv = UCGate(gate_list, up_to_diagonal=self.up_to_diagonal)
        inv.name = name
        inv.definition = definition
        
        return inv

@mtreinish mtreinish removed priority: high stable backport potential The bug might be minimal and/or import enough to be port to stable labels Aug 19, 2022
@mtreinish mtreinish removed this from the 0.21.2 milestone Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants