Conversation
| qubit_remapping = dict(zip(nested_instr.qargs, nested_instr.op.body.qubits)) | ||
|
|
||
| remapped_template_state = builder.template_state.remap(qubit_remapping, idx) | ||
| remapped_template_state.add_stretches(nested_instr.op.body.iter_stretches()) |
There was a problem hiding this comment.
move this to argument of remap?
ihincks
left a comment
ihincks
left a comment
There was a problem hiding this comment.
Thanks, Josh. I have just one question that I left a comment for.
Out of scope, but the exact same problem will exist for variables/expressions in the circuit. It has a parallel API to stretch.
| stretch_map = {} | ||
| for stretch in circuit.iter_stretches(): | ||
| new_stretch = Stretch.new(stretch.name) | ||
| stretch_map[stretch] = new_stretch | ||
| template_circuit.add_declared_stretch(new_stretch) |
There was a problem hiding this comment.
Is there any advantage to making new stretches here? It seems that we could just reuse them, and save having to maintain our own cache.
There was a problem hiding this comment.
In the outer scope we could but will need to make new ones to update the names in inner scopes to account for shadowing.
The cache is mostly for convenience. DAGCircuit doesn't have as rich an API as QuantumCircuit for variable handling, specifically it doesn't have a getter. I'm happy to update to use DAGCircuit.iter_stretches and match if you'd prefer!
ihincks
left a comment
ihincks
left a comment
There was a problem hiding this comment.
Summary: Adds support for Delay instructions with Stretch durations in the build pipeline. Core change is a stretch_map on TemplateState with identity-mapping for outer-scope stretches and scope-indexed names for box-scoped ones, plus a new branch in append_remapped_gate to rewrite Delay(stretch) nodes.
Testing: test_stretch, test_rescoped_stretch, and test_box_stretches cover the main cases well.
Overall: Clean implementation with good test coverage. Two non-blocking comments inline.
| Args: | ||
| stretches: An iterable of stretches to add. | ||
| """ | ||
| prefix = ".".join(str(i) for i in self.scope_idx) |
There was a problem hiding this comment.
suggestion: Name-collision risk with dotted stretch names. Using . as both the scope-level separator and the join character means a stretch named "1.x" in scope [0] produces the same template name ("0.1.x") as a stretch named "x" in scope [0, 1]. Using a non-. separator (e.g. ":") for scope levels would eliminate this class of collision entirely.
| for idx, (instr, name) in enumerate(zip(template, expected_names)): | ||
| assert instr.name == name, f"Instruction {idx}" | ||
|
|
||
| def test_box_stretches(self): |
There was a problem hiding this comment.
Missing a couple of test cases:
- A stretch declared in the outer circuit and referenced inside a box body (not redeclared in the box). The implementation should handle this correctly since outer-scope stretches are identity-mapped in
construct_for_circuit, but there's no test confirming it. - An
IfElseOpbody containingDelay(stretch)— exercises theremap_subcircuitpath.
2 participants
Summary
Closes #383.
Details and comments