Add Surface Code #7

justinlietz · 2024-11-21T22:43:07Z

This PR adds the cudaq::qec::surface_code namespace which includes the surface_code type and the stabilizer_grid helper class. The stabilizer_grid generates the 2d layout of the stabilizers, and which data qubits each stabilizer has support on. Lastly it generates the stabilizers and observables as vectors of cudaq::spin_op which the qec::code expects.

copy-pr-bot · 2024-11-21T22:43:10Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

libs/qec/lib/codes/surface_code_device.cpp

boschmitt · 2024-11-22T17:41:17Z

/ok to test

bmhowe23

Thanks, Justin. I have a few comments/questions below.

libs/qec/include/cudaq/qec/codes/surface_code.h

bmhowe23 · 2024-12-12T17:53:56Z

libs/qec/include/cudaq/qec/codes/surface_code.h

+
+/// @brief enumerates the role of a grid site in the surface codes stabilizer
+/// grid
+enum surface_role { amx, amz, empty };


Just curious: is this "a measure x" and "a measure z", or something else?

That is a fine interpretation, although in my head I was thinking "ancilla for mx/z".

libs/qec/include/cudaq/qec/codes/surface_code.h

bmhowe23 · 2024-12-12T17:57:22Z

libs/qec/include/cudaq/qec/codes/surface_code.h

+stabilizer(patch p, const std::vector<std::size_t> &x_stabilizers,
+           const std::vector<std::size_t> &z_stabilizers);


I didn't think we supported pass-by-reference going into a __qpu__ function?

What is the expected behavior of passing a ref in? This seems to work here as well as in the Steane code.

In this case, either pass by value or pass by ref get you the same thing - a subtlety of the ASTBridge MLIR generation. No matter what the signature is, if you pass a std::vector it is modeled as a cc.stdvec which lowers to a span-like type (a pointer and a size). You are always passing by reference no matter what. Eric can elaborate more if needed.

bmhowe23 · 2024-12-12T18:06:27Z

libs/qec/unittests/test_qec.cpp

+    cudaqx::tensor<uint8_t> Lz = surf_code->get_observables_z();
+
+    {
+      // This is just a regression check, this has not been hand tested


What exactly does this comment mean? Where did the data come from?

The data is only generated from itself, just scraped from the terminal and put into a vector here. Other instances of checking against hardcoded bit strings were hand checked to be correct. This one was not, as there are not printed instances of surface code PCMs I could find.

This comment is to make us aware that these are not magic numbers that we must always match. Various permutations would all be valid PCMs of the surface code.

bmhowe23 · 2024-12-12T18:06:39Z

libs/qec/unittests/test_qec.cpp

+          EXPECT_EQ(t.at({i, j}), parity.at({i, j}));
+    }
+    {
+      // This is just a regression check, this has not been hand tested


Same question here.

bmhowe23 · 2024-12-12T18:06:56Z

libs/qec/unittests/test_qec.cpp

+          EXPECT_EQ(t.at({i, j}), parity_x.at({i, j}));
+    }
+    {
+      // This is just a regression check, this has not been hand tested


Same question here.

bmhowe23

Notes from our meeting today.

bmhowe23 · 2024-12-18T21:15:27Z

libs/qec/include/cudaq/qec/codes/surface_code.h

+
+/// @brief Generates and keeps track of the 2d grid of stabilizers in the
+/// rotated surface code
+// Grid layout is arranged from left to right, top to bottom (row major storage)


We need to double check that the docs are rendering as desired for // vs ///.

bmhowe23 · 2024-12-18T21:15:48Z

libs/qec/include/cudaq/qec/codes/surface_code.h

+// The grid length of 4 corresponds to a distance 3 surface code, which results
+// in: e(0,0)  e(0,1)  Z(0,2)  e(0,3) X(1,0)  Z(1,1)  X(1,2)  e(1,3) e(2,0)
+// X(2,1)  Z(2,2)  X(2,3) e(3,0)  Z(3,1)  e(3,2)  e(3,3)


Double-check formatting.

bmhowe23 · 2024-12-18T21:17:23Z

libs/qec/include/cudaq/qec/codes/surface_code.h

+//
+// The data qubits are located at the four corners of each of the weight-4
+// stabilizers. They are also organized with index increasing from left to
+// right, top to bottom: d0  d1  d2 d3  d4  d5 d6  d7  d8


Double check formatting (two spaces vs single spac)

bmhowe23 · 2024-12-18T21:32:53Z

libs/qec/include/cudaq/qec/codes/surface_code.h

+bool operator<(const vec2d &lhs, const vec2d &rhs);
+
+/// @brief Generates and keeps track of the 2d grid of stabilizers in the
+/// rotated surface code


Perhaps consider adding link to a reference paper who's convention you're following w.r.t. the grid.

bmhowe23 · 2024-12-18T21:34:37Z

libs/qec/lib/codes/surface_code.cpp

@@ -0,0 +1,396 @@
+/****************************************************************-*- C++ -*-****


Nit:

Suggested change

/****************************************************************-*- C++ -*-****

/*******************************************************************************

bmhowe23 · 2024-12-18T21:35:11Z

libs/qec/lib/codes/surface_code_device.cpp

@@ -0,0 +1,87 @@
+/****************************************************************-*- C++ -*-****


Nit:

Suggested change

/****************************************************************-*- C++ -*-****

/*******************************************************************************

bmhowe23 · 2025-01-16T04:27:30Z

/ok to test

bmhowe23 · 2025-01-24T04:13:58Z

/ok to test

bmhowe23 · 2025-01-25T15:21:02Z

I don't know if you're seeing this in your local builds or not, but the CI is reporting some potentially relevant warnings:
https://github.com/NVIDIA/cudaqx/actions/runs/12943122467/job/36166508865?pr=7#step:10:232

E.g., this is repeated many times:

In file included from /__w/cudaqx/cudaqx/libs/qec/unittests/test_qec.cpp:14:
/__w/cudaqx/cudaqx/libs/qec/include/cudaq/qec/codes/surface_code.h:164:23: warning: ‘annotate’ attribute directive ignored [-Wattributes]
  164 | __qpu__ void x(patch p);

justinlietz · 2025-01-25T18:49:29Z

I don't know if you're seeing this in your local builds or not, but the CI is reporting some potentially relevant warnings: https://github.com/NVIDIA/cudaqx/actions/runs/12943122467/job/36166508865?pr=7#step:10:232

E.g., this is repeated many times:
In file included from /__w/cudaqx/cudaqx/libs/qec/unittests/test_qec.cpp:14:
/__w/cudaqx/cudaqx/libs/qec/include/cudaq/qec/codes/surface_code.h:164:23: warning: ‘annotate’ attribute directive ignored [-Wattributes]
  164 | __qpu__ void x(patch p);

I do not see this locally, but I build with -DCMAKE_CXX_FLAGS="-Wno-attributes". Turning this flag off, I do now see this warning, but I'm not sure why surface_code is any different than steane in this regard.

bmhowe23 · 2025-01-25T20:45:47Z

I don't know if you're seeing this in your local builds or not, but the CI is reporting some potentially relevant warnings: https://github.com/NVIDIA/cudaqx/actions/runs/12943122467/job/36166508865?pr=7#step:10:232
E.g., this is repeated many times:
In file included from /__w/cudaqx/cudaqx/libs/qec/unittests/test_qec.cpp:14:
/__w/cudaqx/cudaqx/libs/qec/include/cudaq/qec/codes/surface_code.h:164:23: warning: ‘annotate’ attribute directive ignored [-Wattributes]
  164 | __qpu__ void x(patch p);
I do not see this locally, but I build with -DCMAKE_CXX_FLAGS="-Wno-attributes". Turning this flag off, I do now see this warning, but I'm not sure why surface_code is any different than steane in this regard.

It turns out this warning is only coming from the test_qec.cpp file, not our actual libraries. Our libs are built with -Wno-attributes, but our tests are not. test_qec.cpp does not #include the Steane code, so the tests are not currently reporting this warning. The prior QEC tests just used the pure QEC interfaces. I think we should add add_compile_options(-Wno-attributes) to libs/qec/unittests/CMakeLists.txt (similar to how we already do that in libs/qec/lib/CMakeLists.txt).

justinlietz · 2025-01-26T17:40:02Z

/ok to test

justinlietz requested a review from amccaskey November 21, 2024 22:43

amccaskey reviewed Nov 22, 2024

View reviewed changes

libs/qec/lib/codes/surface_code_device.cpp Show resolved Hide resolved

bmhowe23 reviewed Dec 12, 2024

View reviewed changes

bmhowe23 reviewed Dec 18, 2024

View reviewed changes

justinlietz force-pushed the surface_code branch from 554227b to dc2b76e Compare January 16, 2025 01:56

justinlietz added 11 commits January 24, 2025 03:41

surface code utility functions added

062a00c

moved into new class and cleaned up printing

73330f4

re-inserted offset

119f2a9

added comments and tests

cc940f4

add surface code to docs api

b25cf96

going to just apply logical gates to all qubits for now

fe255bd

updated comment

46e1841

applied changes from code walkthrough meeting

7001fd8

trimming down to just transversal ops for now

b1be66f

removed non-transversal gates, as well as hadamard.

b78ad77

clang formatting

1f64a53

justinlietz force-pushed the surface_code branch from 9242136 to 1f64a53 Compare January 24, 2025 03:41

Added compiler flag to unittests

6d29dde

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Surface Code #7

Add Surface Code #7

justinlietz commented Nov 21, 2024

copy-pr-bot bot commented Nov 21, 2024

boschmitt commented Nov 22, 2024

bmhowe23 left a comment

bmhowe23 Dec 12, 2024

justinlietz Dec 12, 2024

bmhowe23 Dec 12, 2024

justinlietz Dec 12, 2024

amccaskey Dec 12, 2024

bmhowe23 Dec 12, 2024

justinlietz Dec 12, 2024

bmhowe23 Dec 12, 2024

bmhowe23 Dec 12, 2024

bmhowe23 left a comment

bmhowe23 Dec 18, 2024

bmhowe23 Dec 18, 2024

bmhowe23 Dec 18, 2024

bmhowe23 Dec 18, 2024

bmhowe23 Dec 18, 2024

bmhowe23 Dec 18, 2024

bmhowe23 commented Jan 16, 2025

bmhowe23 commented Jan 24, 2025

bmhowe23 commented Jan 25, 2025

justinlietz commented Jan 25, 2025

bmhowe23 commented Jan 25, 2025

justinlietz commented Jan 26, 2025

		stabilizer(patch p, const std::vector<std::size_t> &x_stabilizers,
		const std::vector<std::size_t> &z_stabilizers);

		@@ -0,0 +1,396 @@
		/***************************************************************-- C++ --***

		@@ -0,0 +1,87 @@
		/***************************************************************-- C++ --***

Add Surface Code #7

Are you sure you want to change the base?

Add Surface Code #7

Conversation

justinlietz commented Nov 21, 2024

copy-pr-bot bot commented Nov 21, 2024

boschmitt commented Nov 22, 2024

bmhowe23 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmhowe23 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmhowe23 commented Jan 16, 2025

bmhowe23 commented Jan 24, 2025

bmhowe23 commented Jan 25, 2025

justinlietz commented Jan 25, 2025

bmhowe23 commented Jan 25, 2025

justinlietz commented Jan 26, 2025