Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

RippleCarryAdderD update #358

Merged
merged 4 commits into from
Oct 26, 2020
Merged

Conversation

TviNet
Copy link

@TviNet TviNet commented Oct 25, 2020

PR for issue #335
Implemented controlled adder blocks following this paper but use the sum and carry blocks to compute the output carry bit.

@ghost
Copy link

ghost commented Oct 25, 2020

CLA assistant check
All CLA requirements met.

@msoeken msoeken self-requested a review October 25, 2020 14:15
Copy link
Member

@msoeken msoeken left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your pull request @TviNet. It looks already very good. I left some comments.

/// `summand1` and `summand2`.
/// ## carryOut
/// Carry-out qubit, will be xored with the higher bit of the sum.
operation AdderBlockUsingTemporaryLogicalAnd(carryIn: Qubit, summand1: Qubit, summand2: Qubit, carryOut: Qubit)
Copy link
Member

Choose a reason for hiding this comment

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

This operation can be declared internal as it is only used for the implementation of RippleCarryAdderD.

/// ## carryOut
/// Carry-out qubit, will be xored with the higher bit of the sum.
operation AdderBlockUsingTemporaryLogicalAnd(carryIn: Qubit, summand1: Qubit, summand2: Qubit, carryOut: Qubit)
: Unit is Adj + Ctl {
Copy link
Member

Choose a reason for hiding this comment

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

This operation does not need to be Ctl, since it is only called inside a within block. Then the controlled blocks can be removed and also the body (...) line can be removed, as it will be the only specialisation (the adjoint can be generated automatically).

Carry(auxRegister[idx], xs![idx], ys![idx], auxRegister[idx+1]); // (1)
using (auxRegister = Qubit[nQubits-1]) {
within {
ApplyAnd (xs![0], ys![0], auxRegister[0]);
Copy link
Member

Choose a reason for hiding this comment

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

It should be possible to merge these nested within / apply blocks by moving this ApplyAnd operation call before the for loop in the nested within block.

apply {
within {
for (idx in 1..(nQubits-2)) {
(Controlled AdderBlockUsingTemporaryLogicalAnd) (controls, (auxRegister[idx-1], xs![idx], ys![idx], auxRegister[idx]));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(Controlled AdderBlockUsingTemporaryLogicalAnd) (controls, (auxRegister[idx-1], xs![idx], ys![idx], auxRegister[idx]));
AdderBlockUsingTemporaryLogicalAnd(auxRegister[idx-1], xs![idx], ys![idx], auxRegister[idx]);

This statement does not need to be controlled, as it is inside a within block.

Copy link
Author

@TviNet TviNet Oct 25, 2020

Choose a reason for hiding this comment

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

Thanks for the suggestions
I am using this figure as reference.
image

The block can be considered to have two parts: ComputeTempCarry and EraseTempCarry.
To make use of within..apply structure I have used

AdderBlockUsingTemporaryLogicalAnd =  ComputeTempCarry 
Adjoint AdderBlockUsingTemporaryLogicalAnd  = EraseTempCarry

Since the control only affects EraseTempCarry I had to add a Controlled + Adjoint variant.

I realize this has gotten a bit messy and is probably an incorrect usage of adjoint

So I am planning to use the two blocks ComputeTempCarry is Adj and EraseTempCarry is Adj + Ctl but this would not make use of within..apply

}
}
apply {
(Controlled Carry) (controls, (auxRegister[nQubits-2], xs![nQubits-1], ys![nQubits-1], carry));
Copy link
Member

Choose a reason for hiding this comment

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

This Carry implementation uses two CCNOT gates and could be replaced by something that uses a single ApplyAnd. Could AdderBlockUsingTemporaryLogicalAnd be used?

Copy link
Author

Choose a reason for hiding this comment

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

AdderBlockUsingTemporaryLogicalAnd has to erase the carry bit by design so I don't think it can be used here unless I understand incorrectly. The first CCNOT of Carry can be changed to ApplyAnd since the CarryOut is zero. I haven't been able to find another implementation that would do better.

}
(Controlled CNOT) (controls,(xs![0], ys![0]));
Copy link
Member

Choose a reason for hiding this comment

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

It is not clear to me why this line is here. Could you add some line of documentation to make it clearer.

Copy link
Author

Choose a reason for hiding this comment

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

Since the first bit has CarryIn always zero the operations on first bit can be simplified to

ApplyAnd (xs![0], ys![0], auxRegister[0]);
---<Compute other bits using auxRegister[0]>---
(Adjoint ApplyAnd) (xs![0], ys![0], auxRegister[0]);
(Controlled CNOT) (controls,(xs![0], ys![0]));

to reduce the gate count.

@msoeken msoeken changed the base branch from main to feature/adder October 26, 2020 07:47
Copy link
Member

@msoeken msoeken left a comment

Choose a reason for hiding this comment

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

Thanks for your changes and the clarification. I approve this PR and changed the target to a feature branch.

Thanks for your work on this @TviNet

@msoeken msoeken added the Hacktoberfest-Accepted Contribution accepted for consideration in Hackathon 2020. label Oct 26, 2020
@msoeken msoeken merged commit dcf75e1 into microsoft:feature/adder Oct 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Hacktoberfest-Accepted Contribution accepted for consideration in Hackathon 2020.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants