feat: add quantum memory layout for quantum data types#137
Conversation
Doomsk
left a comment
There was a problem hiding this comment.
Hi @ssmswapnil thank you for this PR. I asked some questions. Let me know if you have any questions or points to make before we move on.
| pub struct QuantumPrimitiveLayout { | ||
| pub qubits: u32, | ||
| } |
There was a problem hiding this comment.
what will happen when new quantum primitive types are added with the same number of qubits?
There was a problem hiding this comment.
The layout only cares about how many qubits to allocate - it doesn't track which primitive type it came from. So yeah, if two different primitives both need 2 qubits, they'd have identical layouts. But that's okay at this layer since the type identity is still preserved through the Ty enum, which is what keys the cache. So @u2 and some future @i2 would be separate cache entries even if their qubit count is the same.
If we ever need the layout to carry type info too (like for different gate behavior per primitive), we could add a field for that. Happy to add it now if you think it makes sense.
There was a problem hiding this comment.
how would you identify which sub-value is being measured or not at the Q3L code from this layout? I want the author to answer it instead of the LLM.
There was a problem hiding this comment.
At the moment there's no connection between the qubit ranges stored in QuantumMemberLayout and what's recorded through cast_attribute(). The layout information can tell us which qubits belong to a particular member using the offsets, but cast_attribute() itself is only keeping a count of measurements and doesn't know which attribute was actually measured.I feel like a better approach could be for cast_attribute() to take the member's SymbolId and record which attribute was measured instead of simply incrementing a counter.
| pub struct QuantumStructLayout { | ||
| pub qubits: u32, | ||
| pub members: Vec<QuantumMemberLayout>, | ||
| } |
There was a problem hiding this comment.
how do you know which quantum variable is this struct?
There was a problem hiding this comment.
You don't, not from the layout alone. I followed how the classical StructLayout does it, which also doesn't store a name (just size, align, members). The identity lives in the cache key: Ty::Struct(TyStruct) has the name: SymbolId, so you always look up layout through the type.
That said, having the name directly on the struct layout could be more practical so you don't need the cache context to identify it. I can add a name: SymbolId field if you'd prefer that.
| pub struct QuantumProgram { | ||
| pub root_layout: QuantumTypeLayout, | ||
| pub data_qubits: u32, | ||
| pub ancilla_qubits: u32, | ||
| pub classical_register_bits: u32, | ||
| } |
There was a problem hiding this comment.
can you explain the logic you used to come up with this struct for QuantumProgram?
There was a problem hiding this comment.
I basically followed the variable lifecycle from the issue - declare, assign, cast:
data_qubitscomes from the type. Declare@v:@bell_tand the type tells you it's 2 qubits.ancilla_qubitscomes from instructions. Something like@syncmight need extra scratch qubits on top of the data register, so those get accumulated as you apply instructions.classical_register_bitscomes from casting. When you measure an attribute, that's one classical output bit. Maps to thecregin the OpenQASM output from the issue.
I kept data and ancilla separate instead of just storing a total because the backend names them differently — data qubits get their own named registers like qv_qs and qv_qt from the issue's example, while ancillas are scratch space like anc_v_sync.
There was a problem hiding this comment.
It doesn't make sense to have a field called data_qubits and it holds the size of the quantum layout for the quantum data in the QuantumProgram?
Measuring an attribute will not produce a single u32 value.
There are other attributes lacking in the struct.
There was a problem hiding this comment.
data_qubits is currently just duplicating information that's already available through root_layout.qubits().Right now cast_attribute() assumes every measurement produces a single classical bit, which isn't always true. For example, measuring something like @u4 would require four classical bits. I can change the method so that it really uses the attribute's layout information to determine how many bits are needed rather than always adding one.
One more thing which i noticed is the structure doesn't store which variable is being measured. Including a SymbolId would make that information explicit. I can revise that part of the design as well.
| /// Record that a quantum attribute was cast (measured), adding one | ||
| /// classical bit to the output register. | ||
| /// | ||
| pub fn cast_attribute(&mut self) { | ||
| self.classical_register_bits += 1; | ||
| } |
There was a problem hiding this comment.
how is this method to be used? Can you give a context or example that you envisioned it?
There was a problem hiding this comment.
So say we have @v:@bell_t where @bell_t { @s:@bool, @t:@bool }. After running instructions, the variable gets cast.
If only @v.@t is measured, cast_attribute() gets called once → classical_register_bits = 1. That's the creg cv_ct[1] from the OpenQASM example in the issue.
If both @v.@s and @v.@t are measured, it gets called twice → classical_register_bits = 2, and the backend would allocate a 2-bit classical register.
The idea is the compilation pass that handles casts would call this once per attribute being measured. Right now it's just a counter but it could be extended later to also track which attributes were cast if the backend needs that mapping.
There was a problem hiding this comment.
But why need it? Where do you use it or where should it be used?
There was a problem hiding this comment.
The idea was that the compilation pass responsible for handling casts would call cast_attribute(), allowing the backend to determine how many classical register (creg) bits need to be allocated.
However, after considering the issues you've pointed out such as not tracking the actual attribute being measured and assuming every measurement is one bit I think the current design is too simplistic. It probably needs to be redesigned rather than patched with incremental fixes.
| #[derive(Clone, Debug)] | ||
| pub struct QuantumEnumLayout { | ||
| pub qubits: u32, | ||
| } |
There was a problem hiding this comment.
QuantumPrimitiveLayout, QuantumStructLayout, and QuantumEnumLayout all expose a qubits field, which allows QuantumTypeLayout::qubits() to work uniformly across all variants. Since enums currently always occupy a single qubit, storing the value as a field may not be useful. It could just be treated as a constant, or the struct could instead store information that's more meaningful for enums, such as variant metadata.
|
Hi @ssmswapnil it seems clear to me that you are using LLM to code and to respond. The code implementation and the explanations you are giving are often conflicting with each other. I cannot understand how this follows the AI policy of the repository, as it doesn't look like you own or understand what was done. Let me know if you have any questions before I close this PR. |
|
Hi @Doomsk , I appreciate your feedback. I relied heavily on AI for this one and didn't have a strong enough grasp of the design to back it up. Thanks for taking the time to review. |
Closes #113
Adds
layout/quantum.rstohhat_core— the quantum-side counterpart to the classicalTypeLayout/LayoutCache. Classical layouts count bytes and insert alignment padding; quantum layouts count qubits with no padding at all (we cannot afford wasting qubits).What's inside:
QuantumPrimitiveLayout— each quantum primitive reports its qubit width:@bool→ 1,@u2→ 2,@u3→ 3,@u4→ 4,@u8→ 8. Classical primitives are rejected early with a panic rather than silently returning zero.QuantumStructLayout— walks quantum members back-to-back, summing qubit counts with no alignment or padding. Each member gets aqubit_offsetfor Q3L register naming. Classical members inside a quantum struct are skipped (handled by the classicalLayoutCache).QuantumEnumLayout— 2-variant named enums only, always 1 qubit (first variant = |0⟩, second = |1⟩). Panics if variant count ≠ 2.QuantumLayoutCache—HashMap<Ty, QuantumTypeLayout>, mirrors the classicalLayoutCache. Method names follow the classical convention (insert_layout,layout_of,has). For structs,insert_layoutrecursively pre-populates member types before building the struct entry, so nested types resolve in one call.QuantumInstruction— minimal stub withnameandancilla_qubits: u32.QuantumProgram— built from a quantum variable's type layout, tracksdata_qubits(from type),ancilla_qubits(from instructions), andclassical_register_bits(from casts/measurements). I separated data and ancilla rather than storing a total because the Q3L backend needs to distinguish type-required qubits (mapped to variable fields) from gate-required scratch qubits.Files changed:
Classical layout path is untouched. Array quantum layout is
unimplemented!()— not specified in the issue.Tests (19 total) covering all six categories from the issue: all five primitive types + panic on classical, simple struct (bell_t + mixed classical/quantum), simple enum (polarization + side + 3-variant panic), nested structs (2-level + 3-level deep), TypeLayout ↔ QuantumTypeLayout round-trip (primitive + struct), and full QuantumProgram workflows (bell_t with sync + multi-instruction accumulation + multi-cast + cache reuse).
What's next: hooking
QuantumPrograminto the compilation pass that processes casts, expandingQuantumInstructioninto a real instruction enum as Q3L primitives get defined, and array support once quantum arrays are specified.Generative AI/LLM disclosure
I used an AI assistant to help me understand the existing type/layout system (how
Ty,TypeLayout,LayoutCache, andStructLayoutfit together) and to draft parts of the implementation.Code and Logic Architecture/Design:
Code content:
Code review:
Code tests: