Template structure simplification #317
Conversation
It has been a few weeks I am thinking about that, and a discussion with Kabir yesterday finally convinced me that refactoring the Template hierarchy was a good idea. The main reason for that refactor is the uneeded complexity of the old Template code: we only need to represent "simple" templates such as logical qubits and their sides. Being able to represent arbitrary templates using the ComposedTemplate class was nice, but not a requirement. With the need to query templates about their middle lines, sides, and likely other properties in the future that were not easily implementable (or even correctly defined) for all the possible ComposedTemplate instance, removing that intermediary seemed like a necessity. This (rather large, sorry) commit aims at greatly simplifying the Template-related module. The interfaces are left unchanged, which reduces by a lot the possibility of introducing errors with this refactor and reduces the number of changes.
nelimee
added
enhancement
|
I should be able to review this and #305 in the next few days and will give you the feedback as soon as possible. |
There was a problem hiding this comment.
The removal of ComposedTemplate indeed simplifies things a lot. And currently I haven't seen any bottlenecks of this simplification considering the current workflow. A few questions in my mind:
- How do you plan to construct more complex templates like the corners/junctions? I assume that for each "composed template" now we need to implement a rather large
instantiatemethod to specify all the plaquette indices. - I remembered that the complexity of
PiecewiseLinearFunctionmainly comes from the usage inComposedTemplate. If it does not exist anymore, is it possible to simplify thescale.py/interval.pyas well?
|
I'm not sure what kind of complex structures we would need, but I think both corners and junctions can be represented as a block graph? |
|
Yes they can be represented by block graph. But when it comes to instantiate them into circuits, there is the possibility that we need to change the underlying template for more complex circuits to fit in. For example, corner/junctions in space need special care about the plaquettes arrangement for the circuit to be distance-preserving. |
The problem is the same with the current architecture. The main complexity is due to the center
This is very much true, I will try to remove that now. |
Co-authored-by: Yiming Zhang <61700160+inmzhang@users.noreply.github.com>
|
In order to demonstrate that it can be done, I implemented the 4-way junction as a new template. Note that 2/3-way junctions are not implemented yet, and there are enough questions we need to collectively answer on them to justify another PR just for that. |
|
|
Now that I included more tests I think this PR is ready to be reviewed. As soon as it is accepted, I will merge this one and just after merge #318.
|
|
This is also how I see it: a showcase that will need to be checked and improved. Thanks for the quick approval ! |
This is based on #317. This PR introduces several things to the `Template` hierarchy. First, a few details: - Remove `pass` from abstract methods bodies - Introduce a `RectangularTemplate` class that inherits from `Template` and provides a default implementation of `get_midline_plaquettes` for `Template` instances with a rectangular form (e.g. all the templates in `tqec.templates.qubit`). Now the main 2 changes: - Introduction of a `GridTemplate` that allows to quickly represent a grid of several identical `RectangularTemplate` instances. - Introduction of a `get_spatially_distinct_subtemplates` function that computes, from a template, all the spatially-local plaquette arrangements found. A little bit more on `get_spatially_distinct_subtemplates`. It takes an array obtained from calling `Template.instantiate` on any `Template` instance, a `radius` (using the Manhattan distance), and compute all the plaquette arrangements that are found in the instantiation with the provided `radius`. For example, a radius of `1` would lead to "sub-template" looking like (`c` being the center of the ball of radius `1`): ``` x x x x c x x x x ``` whereas a radius of 3 would lead to ``` x x x x x x x x x x x x x x x x x x x x x x x x c x x x x x x x x x x x x x x x x x x x x x x x x ``` Now with a real example, the following code ```py from tqec.templates.qubit import QubitTemplate template = QubitTemplate(k=2) situation, mapping = template.get_spatially_distinct_subtemplates(1) print("Full map:") display_template_from_instantiation(situation) for i, subtemplate in mapping.items(): print(f"Subtemplate {i}:") display_template_from_instantiation(subtemplate) ``` outputs: ``` Full map: 1 2 4 6 3 5 7 12 14 16 13 15 9 18 21 28 24 33 11 20 28 21 31 26 8 17 22 29 25 34 10 19 30 23 32 27 Subtemplate 1: . . . . 1 5 . 7 9 Subtemplate 2: . . . 1 5 6 7 9 10 Subtemplate 3: . . . 5 6 2 9 10 11 Subtemplate 4: . . . 5 6 5 9 10 9 // Cut for readability Subtemplate 16: 6 5 6 10 9 10 9 10 9 Subtemplate 17: 7 9 10 8 10 9 3 13 14 // Cut for readability Subtemplate 33: 10 11 . 9 12 . 10 11 . Subtemplate 34: 10 11 . 9 12 . 14 4 . ``` In theory, if we ignore `0` entries in the sub-templates (represented by `.` above), we should recover exactly the template instantiation by superimposing on each entry of the "Full map" printed first the corresponding sub-template. The goal of implementing such a procedure is to be able to list exhaustively the spatially-local sub-templates that we have to consider in order to compute detectors involving measurements in the center plaquette of such sub-template. What is interesting in this approach is that if we now use a way larger `Template` such as ```py from tqec.templates.qubit import QubitTemplate template = QubitTemplate(k=100) # << WAY LARGER situation, mapping = template.get_spatially_distinct_subtemplates(1) print("Full map:") display_template_from_instantiation(situation) for i, subtemplate in mapping.items(): print(f"Subtemplate {i}:") display_template_from_instantiation(subtemplate) ``` the number of distinct spatially local sub-template is still `34`, and we have in the variable `situation` a description of where these sub-templates are. One of the issue I still have is about naming: I do not like the naming of `situation` to describe a 2-dimensional array filled with sub-template indices, but I cannot find something better. Do anyone have an idea? --------- Co-authored-by: Yiming Zhang <61700160+inmzhang@users.noreply.github.com>
There was a problem hiding this comment.
Do we need new display functions for the refactored Template? I can work on this (make a new issue and close issue #71). I'm still in the process of understanding the refactoring.
There was a problem hiding this comment.
Do we need new display functions for the refactored
Template?
I am not sure. I think it depends nearly entirely on whether or not we plan to eventually have a frontend that will need to visually represent templates. Another point is that it would be nice to have nice looking images for the documentation.
So let's say that we might need it, but it is clearly not sure and not a priority. If you still want to work on it, please feel free to do so :)
It has been a few weeks I am thinking about that, and a discussion with Kabir a few days ago finally convinced me that refactoring the Template hierarchy was a good idea.
The main reason for that refactor is the unneeded complexity of the old
Templatecode: we only need to represent "simple" templates such as logical qubits and their sides. Being able to represent arbitrary templates using theComposedTemplateclass was nice, but not a requirement. With the need to query templates about their middle lines, sides, and likely other properties in the future that were not easily implementable (or even correctly defined) for all the possibleComposedTemplateinstance, removing that intermediary seemed like a necessity.This (rather large, sorry) commit aims at greatly simplifying the
Template-related module. The interfaces are left unchanged, which reduces by a lot the possibility of introducing errors with this refactor and reduces the number of changes.Note that this is only a draft pull request, I really want other maintainer opinions on that before even considering it mergeable.
Note also that both #71 and #274 will be impacted by this refactor.
Finally, this is also an experiment to test another idea for automatic detector computation. I will use that PR as a base for another PR on that, and see if it turns out to be a nice approach.