-
Notifications
You must be signed in to change notification settings - Fork 17
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
🛩️ 🏁 Removes the template specification process and cleans up template class 🐰 #193
Conversation
…-template_border-2024.03.13
Thanks @rryoung98 . This is exactly what I had in mind when we last spoke. Per this morning's discussion, we'll want to be able to produce "plaquette footprints," which aren't the same as plaquette templates per se, hence why we needed to change the frontend to reflect that. I think it would be nice to have the ability to specify the plaquette footprint using the infrastructure that exists within the To summarize, the workflow would look like: Unit cell -> specify footprint (using click-and-drag) -> specify circuit on plaquette, based on the footprint. Additionally, once a footprint has been specified, we could be able to choose it from the library. Task #189 will provide the infrastructure for the library, which can include footprints, completed circuits, and scalable templates. @afowler Do you have any more questions/commentary here? In particular, how would we define a template, now that we have footprints defined? |
For the moment they should be only one footprint for everything. For the
moment we don't plan to combine plaquettes with different footprints in a
single circuit.
…On Wed, Mar 13, 2024, 11:01 AM Sam Burdick ***@***.***> wrote:
Thanks @rryoung98 <https://github.com/rryoung98> . This is exactly what I
had in mind when we last spoke.
Per this morning's discussion, we'll want to be able to produce "plaquette
footprints," which aren't the same as plaquette templates per se, hence why
we needed to change the frontend to reflect that.
I think it would be nice to have the ability to specify the plaquette
footprint using the infrastructure that exists within the Template class,
which we can then rename.
To summarize, the workflow would look like:
Unit cell -> specify footprint (using click-and-drag) -> specify circuit
on plaquette, based on the footprint.
Additionally, once a footprint has been specified, we could be able to
choose it from the library. Task #189
<#189> will provide the
infrastructure for the library, which can include footprints, completed
circuits, and scalable templates.
@afowler <https://github.com/afowler> Do you have any more
questions/commentary here? In particular, how would we define a template,
now that we have footprints defined?
—
Reply to this email directly, view it on GitHub
<#193 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKAXTG6M2RQ5YGRL3KPKM3YYCH73AVCNFSM6AAAAABEUMNFQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJVGIYDENBYG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, but making some suggestions based on this morning's meeting.
@@ -263,13 +128,9 @@ export default class Template { | |||
} | |||
// Render the plaquette | |||
const plaquette = new Plaquette(this.selectedQubits, this.workspace, this.app); | |||
// Remove seleected qubits from the template qubits, so they can be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, I don't think a Footprint
should remember the specific set of qubits (in the viewport) used to create it, but should remember a cell of qubit positions (just like the unit cell) for re-creating the footprint anywhere on the viewport. Can we add that to this class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, would you mind clarifying how we should go about the positioning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When it is placed, that the absolute position obviously must known to the workspace to draw its position. When it is stored (say, in redux), we only need to store the relative positions. There's a 1-1 correspondence, and storing relative positions helps us keep things general, such as when a new plaquette is added from the library, or an existing workspace created by the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay makes sense, w.r.t. relative position could you point me towards any existing code that lays out the foundation?
@afowler OK, we can leave footprint specification as-is then. I'll add plaquette footprint saving/specification to the next milestone. |
The best example of the difference between a footprint and a plaquette is
our usual architecture and a memory surface code. The footprint of every
plaquette will be a square including 5 qubits. The plaquette itself, and
it's shape symbol, will fit inside this footprint, and could be a subset,
for example a triangle that attaches to the bottom, top, right, or left of
the footprint. Every plaquette should have the same footprint so that
plaquettes connect neatly when inserted into templates which are just
integer arrays.
Does this clarify the need for a footprint?
…On Wed, Mar 13, 2024, 11:12 AM Sam Burdick ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Looks good overall, but making some suggestions based on this morning's
meeting.
------------------------------
In frontend/src/plaquettes/Template.js
<#193 (comment)>:
> }
- // Render the template control buttons
- renderTemplateControlButtons() {
We can still keep this functionality. That goes for most of the deleted
code here
------------------------------
In frontend/src/plaquettes/Template.js
<#193 (comment)>:
> import Plaquette from './Plaquette';
import notification from '../components/notification';
import Button from '../components/Button';
+/**
+ * Template class to create the template for the plaquettes
+ * @Class Template
+ * @param {Container} workspace - The workspace container
+ * @param {PIXI.Application} app - The PIXI application
+ * @param {number} x - The x coordinate
+ * @param {number} y - The y coordinate
+ * @returns {void}
+ */
export default class Template {
rename to class Footprint.
------------------------------
In frontend/src/plaquettes/Template.js
<#193 (comment)>:
> this.app.view.addEventListener('click', this.selectQubit);
- this.plaquetteButton.visible = true;
- this.plaquetteButton.on('click', () => {
- // Create the plaquettes and tile
- this.createPlaquette();
Footprint can still create a plaquette on top of it. In fact, I'm not sure
I fully understand the difference between plaquettes and footprints at this
point. @afowler <https://github.com/afowler> Do you have any ideas?
------------------------------
In frontend/src/plaquettes/Template.js
<#193 (comment)>:
> }
};
- // Create the plaquette from the selected qubits and assign it to the template
+ /**
+ * Create the plaquette from the selected qubits and assign it to the template
assign it to the Footprint? Again, I still need to understand the
difference.
------------------------------
In frontend/src/plaquettes/Template.js
<#193 (comment)>:
> @@ -263,13 +128,9 @@ export default class Template {
}
// Render the plaquette
const plaquette = new Plaquette(this.selectedQubits, this.workspace, this.app);
- // Remove seleected qubits from the template qubits, so they can be
To be clear, I don't think a Footprint should remember the specific set
of qubits (in the viewport) used to create it, but should remember a cell
of qubit positions (just like the unit cell) for re-creating the footprint
anywhere on the viewport. Can we add that to this class?
------------------------------
In frontend/src/plaquettes/Template.js
<#193 (comment)>:
> this.startX = 0;
this.startY = 0;
this.plaquette = null;
- this.plaquetteButton = plaquetteButton;
- this.templateQubits = selectedQubits || [];
+ this.selectQubitsButton = new Button('Select 3 qubits to make a Plaquette', x, y + 50);
Before this point, qubits should not be clickable (when my mouse hovers
over them, the Mickey Mouse hand should not appear). Can we change that?
------------------------------
In frontend/src/plaquettes/Template.js
<#193 (comment)>:
> this.app.view.addEventListener('click', this.selectQubit);
- this.plaquetteButton.visible = true;
- this.plaquetteButton.on('click', () => {
- // Create the plaquettes and tile
- this.createPlaquette();
- this.workspace.addChild(this.container);
- // Clear the selected qubits
- this.selectedQubits = [];
- notification(this.app, 'Step 4: Define the circuit');
+ this.unselectQubitsButton.visible = true;
+ this.selectQubitsButton.visible = false;
+ this.createPlaquetteButton.visible = false;
+ });
+ this.unselectQubitsButton.on('click', () => {
Nice. I'm planning on adding undo/redo buttons in general (for the whole
app), but this is good to have until that's implemented.
------------------------------
In frontend/src/plaquettes/Template.js
<#193 (comment)>:
> this.startX = 0;
this.startY = 0;
this.plaquette = null;
- this.plaquetteButton = plaquetteButton;
- this.templateQubits = selectedQubits || [];
+ this.selectQubitsButton = new Button('Select 3 qubits to make a Plaquette', x, y + 50);
Also, it should say "Select 3+ qubits to make a Plaquette Footprint"
—
Reply to this email directly, view it on GitHub
<#193 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKAXTB64AW2ZT3YWYDMMBDYYCJKLAVCNFSM6AAAAABEUMNFQGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMZUHA3DKNZXGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
To be clear, the entire surface code that the user is building has only one footprint? @afowler That makes sense, but would the triangular plaquette still be something we want in the plaquette library? What would the use case for creating one be? I think the purpose of the library would be to have different circuits. |
The triangle would just be a visual mnemonic. It would help you remember
that this circuit only touched those three qubits, for example.
…On Wed, Mar 13, 2024, 11:51 AM Sam Burdick ***@***.***> wrote:
could be a subset, for example a triangle that attaches to the bottom,
top, right, or left of the footprint.
@afowler <https://github.com/afowler> That makes sense, but would the
triangular plaquette still be something we want in the plaquette library?
What would the use case for creating one be? I think the purpose of the
library would be to have different *circuits*.
—
Reply to this email directly, view it on GitHub
<#193 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKAXTDPIS7TN6SHU37MKPTYYCNZTAVCNFSM6AAAAABEUMNFQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJVGM3TENBYGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@rryoung98 Is this ready for re-review? |
@smburdick i haven't done the relative positioning since the frontend version 2, if i recall might already have the relative positioning notion. Otherwise, it's all set for re-review |
Thanks, and in the future, be sure to request that from the reviewer when ready. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments.
@smburdick please re-review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should remove this label as a button, since I don't have to actually click it to make a new footprint -- I can still make plaquettes by clicking on qubits, and clicking "create plaquette". So, instead of having the button, we can simply add the text to the topbar.
Looks good otherwise, you can make the changes here, or create a new issue to address that.
Closes #186