Skip to content

C3XGate(angle=...) deprecation#4378

Closed
1ucian0 wants to merge 37 commits into
Qiskit:masterfrom
1ucian0:c3x_angle
Closed

C3XGate(angle=...) deprecation#4378
1ucian0 wants to merge 37 commits into
Qiskit:masterfrom
1ucian0:c3x_angle

Conversation

@1ucian0
Copy link
Copy Markdown
Member

@1ucian0 1ucian0 commented May 2, 2020

Summary

The C3XGate class supports an angle parameter mostly because the need of a C3SqrtXGate. This turns C3XGate into a generic C3RXGate (which support angle) and makes C3SqrtXGate and C3XGate subclasses of C3RXGate.

Details and comments

Comment thread qiskit/circuit/library/standard_gates/x.py Outdated
@1ucian0 1ucian0 marked this pull request as draft May 2, 2020 18:24
Comment thread qiskit/circuit/library/standard_gates/x.py Outdated
Comment thread qiskit/circuit/library/standard_gates/x.py Outdated
@Cryoris Cryoris marked this pull request as ready for review June 30, 2020 14:32
@Cryoris Cryoris requested a review from a team as a code owner June 30, 2020 14:32
string (e.g. '110'), or None. If None, use all 1s.
"""
super().__init__('c3sqrtx', 4, [], label=label, num_ctrl_qubits=3, ctrl_state=ctrl_state)
self.base_gate = Gate('sqrtx', 1, [])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did we agree to give this gate it's own class in this PR?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If we have the SXGate base class it depends. We need to check if this decomposition is more efficient than the default mechanism to construct the 3-controlled version, also blocked by #4638.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

With #4638 and SXGate().control(3):

>>> transpiled = transpile(circuit, basis_gates=['u1','u2','u3','cx'], optimization_level=0)
>>> transpiled.count_ops()
OrderedDict([('cx', 120), ('u1', 112), ('u3', 30), ('u2', 4)])
>>> transpiled = transpile(circuit, basis_gates=['u1','u2','u3','cx'], optimization_level=3)
>>> transpiled.count_ops()
OrderedDict([('cx', 100), ('u1', 98), ('u3', 2), ('u2', 1)])

the decomposition in this PR

>>> transpiled = transpile(circuit, basis_gates=['u1','u2','u3','cx'], optimization_level=0)
>>> transpiled.count_ops()
OrderedDict([('u1', 21), ('cx', 20), ('u2', 14)])
>>> transpiled = transpile(circuit, basis_gates=['u1','u2','u3','cx'], optimization_level=3)
>>> transpiled.count_ops()
OrderedDict([('u1', 20), ('cx', 20), ('u2', 2)])

Looking at the CX count I think we should add this class.

"""
from .u1 import CU1Gate

controlled_v = Instruction('cv', 2, 0, [])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could go into the new module for sqrtx as well.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It should, waiting for it to be merged 👍

@1ucian0
Copy link
Copy Markdown
Member Author

1ucian0 commented Jul 2, 2020

on hold until #4638 is merged.

@1ucian0 1ucian0 added the on hold Can not fix yet label Jul 2, 2020
@1ucian0
Copy link
Copy Markdown
Member Author

1ucian0 commented Aug 12, 2020

#4638 merged. removing on hold

@1ucian0 1ucian0 removed the on hold Can not fix yet label Aug 12, 2020
# [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0],
# [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0],
# [0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0]], dtype=complex)
def to_matrix(self):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is correct only if angle==pi/4, right?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes. In principle we could compute the matrix for any angle and return that

@1ucian0 1ucian0 marked this pull request as draft September 12, 2020 15:47
@1ucian0
Copy link
Copy Markdown
Member Author

1ucian0 commented Jan 22, 2021

PR #5668 is already taking care of this. Closing.

@1ucian0 1ucian0 closed this Jan 22, 2021
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