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

Add Heavy hex code to generators #293

Merged
merged 30 commits into from
Aug 23, 2021
Merged

Add Heavy hex code to generators #293

merged 30 commits into from
Aug 23, 2021

Conversation

nahumsa
Copy link
Contributor

@nahumsa nahumsa commented Mar 25, 2021

Added implementation of heavy hex, the input is the code distance, d. In the implementation I chose to add nodes referring to each type of qubit used in the Heavy Hex Code (data, syndrome, and flag) which may cause the node order to be confusing, but that was the best way that I figure out how to implement. Now I will explain the way that I implemented the code, which could be better for people without experience with the heavy hex code.

In the heavy hex code, there are d ** 2 data qubits, (d + 1) * (d - 1) / 2 syndrome qubits, and d * (d - 1) flag qubits. In order to implement I follow 3 steps:

    1. Each flag qubit is conneceted with 2 data qubits forming a chain on the graph, the chain has size d.
    1. Connect syndrome qubits that are connected to data qubits.
    1. Connect flag and syndrome qubits.

In steps (ii) and (iii), there are two patterns depending on the eve (odd) columns of syndrome qubits, which are treated accordingly using an if statement. It was also required to use .chunk() for Vectors in order to represent them in a column.

Related to #150

Added firs implementation for heavy hex, it only implements 1 1 heavy hex and has some hardcoded parts that will be worked on.
@nahumsa nahumsa changed the title [WIP] Added Heavy hex to generators Add Heavy hex to generators Apr 23, 2021
@coveralls
Copy link

coveralls commented Apr 24, 2021

Pull Request Test Coverage Report for Build 1159775214

  • 216 of 220 (98.18%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.08%) to 97.514%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/generators.rs 216 220 98.18%
Totals Coverage Status
Change from base Build 1156543312: 0.08%
Covered Lines: 9491
Relevant Lines: 9733

💛 - Coveralls

@nahumsa nahumsa changed the title Add Heavy hex to generators Add Heavy hex code to generators May 12, 2021
@mtreinish mtreinish added this to the 0.10.0 milestone Jun 4, 2021
Copy link
Contributor

@ecpeterson ecpeterson left a comment

Choose a reason for hiding this comment

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

Same as in #313 : no comment on the Rust, quantum stuff all looks good, and I left a few notes on clarifying intent.

);
} else if i % 2 == 1 {
graph.add_edge(
nodes_data[(i + 1) * d - 1],
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as in #313 : I'd prefer this to be written i * d + (d - 1).

);
graph.add_edge(
syndrome_chunk[syndrome_chunk.len() - 1],
nodes_data[(i + 2) * d - 1],
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above.

@@ -983,6 +983,139 @@ pub fn directed_grid_graph(
})
}

/// Generate an undirected heavy hex graph.
Copy link
Contributor

Choose a reason for hiding this comment

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

A useful reference here is Fig 2 of https://arxiv.org/pdf/1907.09528.pdf , and here's some ASCII art:

... D-S-D   D ...
    |   |   |
...-F   F-S-F ...
    |   |   |
... D   D   D ...
    |   |   |
... F-S-F   F-...
    |   |   |
    .........
    |   |   |
... D   D   D ...
    |   |   |
...-F   F-S-F ...
    |   |   |
... D   D   D ...
    |   |   |
... F-S-F   F-...
    |   |   |
    .........
    |   |   |
... D   D   D ...
    |   |   |
...-F   F-S-F ...
    |   |   |
... D   D   D ...
    |   |   |
... F-S-F   F-...
    |   |   |
... D   D-S-D ...

@ecpeterson
Copy link
Contributor

One other note for onlookers, not really germane to the PR: it's easier to understand these lattices as perfect square grids with holes poked in them, rather than assembling the pieces from scratch.

src/generators.rs Outdated Show resolved Hide resolved
nahumsa and others added 2 commits July 28, 2021 16:21
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Copy link
Member

@mtreinish mtreinish 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 doing this. The only question I have before merging is do you think it makes sense to add a directed version too? If so I think that would be useful to have.

@ecpeterson
Copy link
Contributor

Good catch, and up to y'all. It's ambiguous what directionality is preferred by the circuit embodiments of the stabilizer checks, so I don't think it'd be punting to skip this — but I also think that "flag qubits are targets of edges" is the maxim that's compatible with device engineering.

@nahumsa
Copy link
Contributor Author

nahumsa commented Aug 3, 2021

I'll implement the directed version of the heavy hex code in order to be used on the couplingMap class.

Copy link
Member

@mtreinish mtreinish 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 adding the directed variant. I do wonder if we can de-duplicate the implementation since the code is mostly identical but we can do that in a follow up (as it also applies to most of the generators).

Just one inline comment on the edge direction based on @ecpeterson's comment: #293 (comment) and the figures in the paper about the hardware implementation. Once that's fixed I think this is good to go

src/generators.rs Outdated Show resolved Hide resolved
src/generators.rs Outdated Show resolved Hide resolved
src/generators.rs Outdated Show resolved Hide resolved
src/generators.rs Outdated Show resolved Hide resolved
src/generators.rs Outdated Show resolved Hide resolved
src/generators.rs Outdated Show resolved Hide resolved
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

Looking at figure 10 more closely for #313 I think the direction for the data-> syndrome edges:

Screenshot_2021-08-17_13-24-02
Screenshot_2021-08-17_12-58-35

src/generators.rs Outdated Show resolved Hide resolved
src/generators.rs Outdated Show resolved Hide resolved
src/generators.rs Outdated Show resolved Hide resolved
src/generators.rs Outdated Show resolved Hide resolved
@mtreinish mtreinish mentioned this pull request Aug 17, 2021
4 tasks
@lcapelluto lcapelluto self-assigned this Aug 20, 2021
Copy link
Contributor

@ecpeterson ecpeterson left a comment

Choose a reason for hiding this comment

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

With @mtreinish's amendments, this LGTM.

src/generators.rs Outdated Show resolved Hide resolved
src/generators.rs Outdated Show resolved Hide resolved
src/generators.rs Outdated Show resolved Hide resolved
@mtreinish
Copy link
Member

LGTM, I just went ahead and applied the suggestions to get this in for the release. Thanks for doing this @nahumsa and sticking with it for so long

@mtreinish mtreinish merged commit d9d723d into Qiskit:main Aug 23, 2021
@nahumsa
Copy link
Contributor Author

nahumsa commented Aug 23, 2021

Thanks both @mtreinish and @ecpeterson for the review.
Sorry, I didn't update the code quickly.

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.

None yet

5 participants