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

Minor fixes for new adder and multiplier gates classes #13530

Merged
merged 7 commits into from
Dec 12, 2024

Conversation

alexanderivrii
Copy link
Contributor

Summary

This PR adds several fixes and improvements for the new adder and multiplier gate classes
FullAdderGate, HalfAdderGate, ModularAdderGate and MultiplierGate.

First, it adds the _define method to each of these classes, populating self.definition with some default decomposition of the gate that does not use any ancilla qubits and in particular allows to construct Operators from quantum circuits containing these gates.

Second, it fixes the plugins for adder gates based on the "ripple_v95" synthesis function that requires n-1 ancilla qubits. Previously the function could be called with fewer qubits available and the synthesized quantum circuit was not guaranteed to fit into the original quantum circuit leading to internal errors in HighLevelSynthesis pass.

Third, it adds the default synthesis plugin for FullAdderGate and improves the default synthesis plugins for HalfAdderGate and ModularAdderGate to always choose the best synthesis method depending on the number of ancillas available. Previously, we were getting less optimal circuits for certain small values of n.

Details and comments

Added define method to HalfAdder, FullAdder, and ModularAdder
classes to allow constructing Operators from quantum circuits
containing such adder gates.

Fixing plugins related to adder_ripple_v95, the plugins can be used
only if n-1 clean ancillas are available.

Improved the default adder plugins to choose the best decomposition
based on the number of state qubits and the number of ancilla qubits.
@alexanderivrii alexanderivrii requested review from ShellyGarion and a team as code owners December 5, 2024 14:59
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Cryoris
  • @Qiskit/terra-core
  • @ajavadia

@alexanderivrii alexanderivrii added the bug Something isn't working label Dec 5, 2024
@Cryoris Cryoris added this to the 2.0.0 milestone Dec 5, 2024
@alexanderivrii alexanderivrii added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Dec 5, 2024
@coveralls
Copy link

coveralls commented Dec 5, 2024

Pull Request Test Coverage Report for Build 12293335092

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 38 of 39 (97.44%) changed or added relevant lines in 3 files are covered.
  • 25 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.008%) to 88.958%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/transpiler/passes/synthesis/hls_plugins.py 26 27 96.3%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 94.02%
crates/qasm2/src/lex.rs 6 92.48%
crates/circuit/src/circuit_data.rs 18 95.42%
Totals Coverage Status
Change from base Build 12182055039: 0.008%
Covered Lines: 79422
Relevant Lines: 89280

💛 - Coveralls

@kevinhartman
Copy link
Contributor

@Cryoris should we include this for 1.3.1? If so, could you take a look?

@kevinhartman kevinhartman modified the milestones: 2.0.0, 1.3.2 Dec 10, 2024
@alexanderivrii alexanderivrii changed the title Ops from gates Minor fixes for new adder and multiplier gates classes Dec 11, 2024
Copy link
Contributor

@Cryoris Cryoris left a comment

Choose a reason for hiding this comment

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

Two tiny comments on the test/reno, but the fix LGTM 👍🏻

test/python/circuit/library/test_adders.py Outdated Show resolved Hide resolved
releasenotes/notes/fix-adder-gates-39cf3d5f683e8880.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@Cryoris Cryoris left a comment

Choose a reason for hiding this comment

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

LGTM thanks for the updates!

@alexanderivrii alexanderivrii modified the milestones: 1.3.2, 1.3.1 Dec 12, 2024
@alexanderivrii alexanderivrii added this pull request to the merge queue Dec 12, 2024
Merged via the queue into Qiskit:main with commit b9ea266 Dec 12, 2024
17 checks passed
mergify bot pushed a commit that referenced this pull request Dec 12, 2024
* Fixes to AdderGate classes

Added define method to HalfAdder, FullAdder, and ModularAdder
classes to allow constructing Operators from quantum circuits
containing such adder gates.

Fixing plugins related to adder_ripple_v95, the plugins can be used
only if n-1 clean ancillas are available.

Improved the default adder plugins to choose the best decomposition
based on the number of state qubits and the number of ancilla qubits.

* Adding tests for the case that adder plugins do not apply

* Constructing Operators from circuits with MultiplierGates

* release notes

* docstring fix

* apply suggestions from code review

* futher improving tests based on additional review comments

(cherry picked from commit b9ea266)
github-merge-queue bot pushed a commit that referenced this pull request Dec 12, 2024
* Fixes to AdderGate classes

Added define method to HalfAdder, FullAdder, and ModularAdder
classes to allow constructing Operators from quantum circuits
containing such adder gates.

Fixing plugins related to adder_ripple_v95, the plugins can be used
only if n-1 clean ancillas are available.

Improved the default adder plugins to choose the best decomposition
based on the number of state qubits and the number of ancilla qubits.

* Adding tests for the case that adder plugins do not apply

* Constructing Operators from circuits with MultiplierGates

* release notes

* docstring fix

* apply suggestions from code review

* futher improving tests based on additional review comments

(cherry picked from commit b9ea266)

Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants