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

Refactoring improved adder implementation #359

Open
wants to merge 3 commits into
base: feature/adder
Choose a base branch
from

Conversation

msoeken
Copy link
Member

@msoeken msoeken commented Oct 26, 2020

This PRs refactors the recent changes by @TviNet in #358 to address #335.

With these changes the operations Carry and Sum are no longer needed inside the Standard library, but have been exposed as public API, and therefore cannot be removed.

@msoeken msoeken mentioned this pull request Oct 27, 2020
3 tasks
Copy link
Contributor

@guenp guenp left a comment

Choose a reason for hiding this comment

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

I'm not sure I can approve this PR because I will need some more background on the refactor (and possibly an explanation with a whiteboard or so :)) but it looks like you also made some nice code formatting changes!

Standard/src/Arithmetic/Integer.qs Show resolved Hide resolved
CCNOT (carryIn, summand2, carryOut);
CCNOT(summand1, summand2, carryOut);
CNOT(summand1, summand2);
CCNOT(carryIn, summand2, carryOut);
}

/// # Summary
/// Implements a compute carry gate using a temporary logical-AND construction.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this related to figure 1 in this paper (https://arxiv.org/pdf/1709.06648.pdf)? If yes could we link it here?

Copy link
Member Author

Choose a reason for hiding this comment

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

This operation is not related to that paper. It has been used in the previous implementation, and only remains in the code, because it has been exposed as public API. It basically computes the majority-of-three function out-of-place into carryOut, but also changes the state of summand2.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok thanks! Is there another reference we can link here?

Co-authored-by: Guen P <guenp@microsoft.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants