QCHackers / tqec

Design automation software tools for Topological Quantum Error Correction
https://tqec.app
Apache License 2.0
58 stars 15 forks source link

Observables #213

Closed Gistbatch closed 1 month ago

Gistbatch commented 2 months ago

closes: #141

I started to develop this, and I have some open questions:

Todos:

afowler commented 2 months ago

In my opinion there would be very little benefit and significant additional complexity allowing even code distances, so I would support not allowing this at all and consequently always having well defined mid lines.

Best, Austin.

On Mon, Apr 22, 2024, 8:19 AM Philipp Seitz @.***> wrote:

closes: #141 https://github.com/QCHackers/tqec/issues/141

I started to develop this, and I have some open questions:

  • I've implemented a test for the RawRectangleTemplate. @nelimee https://github.com/nelimee, can you clarify if this is the intended behavior?
  • What should the scope of this Issue/PR be? Only build this for the available templates, including moves and so on.
  • How do we define midlines for uneven-sized templates, e.g., where the number of qubit rows is even?

Todos:

  • Add all templates
  • Write tests, including scaling
  • Figure out edge cases: Moving, uneven ...
  • Documentation

You can view, comment on, or merge this pull request online at:

https://github.com/QCHackers/tqec/pull/213 Commit Summary

File Changes

(4 files https://github.com/QCHackers/tqec/pull/213/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/QCHackers/tqec/pull/213, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKAXTAR2AFW3WUIWSIA7CLY6US7VAVCNFSM6AAAAABGS75KBKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI2TMOBSGQYDSMA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

nelimee commented 2 months ago

I've implemented a test for the RawRectangleTemplate. @nelimee, can you clarify if this is the intended behavior?

The output you fixed is dependent on the actual Plaquette implementation. What if the origin (i.e., (0, 0) point) was on the top-left of the Plaquette? Then both the output coordinates would change. I cannot remember if we completely ruled out that case though.

In any case, I think this method needs the Plaquette instances to be able to function correctly and generically.

What should the scope of this Issue/PR be? Only build this for the available templates, including moves and so on.

For this PR, only build this for the available Templates. Moving will come after.

How do we define midlines for uneven-sized templates, e.g., where the number of qubit rows is even?

As Austin said, I would rule out that case. To do that, I would change the default_observable_qubits return type to ty.Sequence[tuple[cirq.GridQubit, int]] | None, returning None being a signal that the midline could not be computed.

Also, I think the implementations of default_observable_qubits might be factorisable. But let's have something working for the moment.

Gistbatch commented 2 months ago

@nelimee, I've updated this; let me know what you think.

If the midline is supposed to depend on the plaquettes, we need this information, so I built observable_qubits_from_template similar to the generate_circuit function. The template now only returns the row and column indices of the templates that have the observable qubits, by convention, above/left of the midline.

nelimee commented 2 months ago

If the midline is supposed to depend on the plaquettes

I do not get how you can generate correct results without any idea of the plaquette used. For the moment, all the plaquettes we use have the support

1   2
  5 
3   4

with:

  1. cirq.GridQubit(-1, -1)
  2. cirq.GridQubit( 1, -1)
  3. cirq.GridQubit(-1, 1)
  4. cirq.GridQubit( 1, 1)
  5. cirq.GridQubit( 0, 0)

but (as far as I remember) we never explicitly said that was the only possible default. This means that a plaquette with the same support but with

  1. cirq.GridQubit(0, 0)
  2. cirq.GridQubit(2, 0)
  3. cirq.GridQubit(0, 2)
  4. cirq.GridQubit(2, 2)
  5. cirq.GridQubit(1, 1)

is possible. If that is the case, then we need to have access to that information, else the midline qubits will be completely wrong. I am missing something here?

I am making a few comments on the code before commenting again on the overall approach.

afowler commented 2 months ago

Some reasonable restrictions. We already restrict ourselves to plaquettes with a rectangular footprint. The midline calculation is associated with a square 2kx2k template of such plaquettes. It would be reasonable to restrict ourselves to plaquettes with exactly four qubits in the four corners of the plaquette, then the midline calculation should depend on the shape of the plaquette, rather than the precise labeling of the qubits in the plaquette.

Best, Austin.

On Fri, Apr 26, 2024, 8:34 AM Adrien Suau @.***> wrote:

If the midline is supposed to depend on the plaquettes

I do not get how you can generate correct results without any idea of the plaquette used. For the moment, all the plaquettes we use have the support

1 2 5 3 4

with:

  1. cirq.GridQubit(-1, -1)
  2. cirq.GridQubit( 1, -1)
  3. cirq.GridQubit(-1, 1)
  4. cirq.GridQubit( 1, 1)
  5. cirq.GridQubit( 0, 0)

but (as far as I remember) we never explicitly said that was the only possible default. This means that a plaquette with the same support but with

  1. cirq.GridQubit(0, 0)
  2. cirq.GridQubit(2, 0)
  3. cirq.GridQubit(0, 2)
  4. cirq.GridQubit(2, 2)
  5. cirq.GridQubit(1, 1)

is possible. If that is the case, then we need to have access to that information, else the midline qubits will be completely wrong. I am missing something here?

I am making a few comments on the code before commenting again on the overall approach.

— Reply to this email directly, view it on GitHub https://github.com/QCHackers/tqec/pull/213#issuecomment-2079626787, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKAXTCWTHPW2B4VAOCLI2TY7JXYDAVCNFSM6AAAAABGS75KBKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZZGYZDMNZYG4 . You are receiving this because you commented.Message ID: @.***>

Gistbatch commented 2 months ago

Using their coordinates, I take the qubits furthest to the right (bottom). This works only for rectangular shapes but has the advantage that this works independently of the orientation. As an alternative, we could implement this as the default behavior in the plaquette, which can be overwritten. I'll clean up the code once we've decided on the mechanism and implement the rest of the templates.

nelimee commented 2 months ago

then the midline calculation should depend on the shape of the plaquette, rather than the precise labeling of the qubits in the plaquette.

I agree, the issue here is that, in the current state of the code, the positions of the plaquette qubits can change all the positions. Basically, the positioning algorithm starts by positioning the origin of the top-left-most plaquette on the qubit (0, 0), all the other qubits position being ultimately determined relative to that position. That mean that in the 2 cases I presented above, all the qubits of the template are shifted by (1, 1). And we need exact coordinates for the midline.

This limitation should be quite easy to remove though, using the top-left corner of the plaquette instead of the origin in the first step would solve it on the current use-case. Finally, this is very much an implementation detail for the moment, but it has implications on what the user see and experience, so we might want to change the current default anyway.

Using their coordinates, I take the qubits furthest to the right (bottom). This works only for rectangular shapes but has the advantage that this works independently of the orientation. As an alternative, we could implement this as the default behavior in the plaquette, which can be overwritten.

Both approaches seems fine for me. The second one is likely more generic, but we are only interested in rectangular plaquette for the moment so this is not really a problem to implement a simpler, less generic, version.

Gistbatch commented 1 month ago

@nelimee, I'm confused about why the test keeps failing. Do you have any idea why this is the case? I can run it locally without this problem, and I'm not sure how to fix this as it seems it works completely fine with circuit_test which has a similar import structure.

nelimee commented 1 month ago

@nelimee, I'm confused about why the test keeps failing. Do you have any idea why this is the case? I can run it locally without this problem, and I'm not sure how to fix this as it seems it works completely fine with circuit_test which has a similar import structure.

This might be due to #198 being merged in main. I am not sure what https://github.com/actions/checkout is checking out, but it might be worth updating this branch with the latest changes from main and see if that resolves the issue.

Gistbatch commented 1 month ago

I've now implemented the midline definition for the existing template instances. For the ComposedTemplate and its children, I'm wondering what the default behavior should be. It seems impossible to know which qubits are on the midline without prior knowledge. For example, the QubitSquareTemplate has a midline, but none of the associated template midlines coincide. We could hard code this, but figuring out which templates to look at for qubits seems arbitrary and complex. @afowler, @nelimee any suggestions?

nelimee commented 1 month ago

It seems at least hard, maybe impossible (there are likely instantiations of the CompositeTemplate that do not represent a valid code, and so do not have observables), to me too. I will review the code as it is right now, let's wait for @afowler answer.

Gistbatch commented 1 month ago

I wrote tests for all the templates now. I don't know what the intended behavior for the stacked template is. Also, I left the CornerTemplate and StackedTemplate unimplemented. I could hardcode the corner template similar to the qubit template if needed. There isn't a good way of knowing what the midline is supposed to be for the stacked template, so I just left it. The only option is to give all possible midlines to the user, and they have to piece them together themselves.

Gistbatch commented 1 month ago

@nelimee I'll merge this once you approve