Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Control flow instructions don't work with circuit_to_instruction #11379

Open
mtreinish opened this issue Dec 7, 2023 · 7 comments · May be fixed by #13921
Open

Control flow instructions don't work with circuit_to_instruction #11379

mtreinish opened this issue Dec 7, 2023 · 7 comments · May be fixed by #13921
Assignees
@1ucian0
Labels
bug Something isn't working mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library
Milestone
2.0.0

Comments

@mtreinish
Copy link
Member

Environment

  • Qiskit Terra version: 1.0.0.dev0+ac7aca3
  • Python version: 3.11
  • Operating system: Linux

What is happening?

Trying to convert a circuit with a control flow operation in it to an instruction using qiskit.converters.circuit_to_instruction fails while processing the control flow condition. It is treating it like a c_if legacy style condition even for control flow ops and this causes it to fail with a NotImplementedError because the function is trying to call c_if.

How can we reproduce the issue?

>>> qc = QuantumCircuit(2)
>>> qc.h(0)
<qiskit.circuit.instructionset.InstructionSet object at 0x7f0813ebc280>
>>> qc.cx(0, 1)
<qiskit.circuit.instructionset.InstructionSet object at 0x7f0813ebef80>
>>> qc.measure_all()
>>> with qc.if_test((qc.clbits[0], 0)):
...     qc.x(0)
>>> circuit_to_instruction(qc)

What should happen?

We either should have a descriptive error saying we can't nest control flow operations in an instruction or update the .condition handling in circuit_to_instruction to understand a control flow operation.

Any suggestions?

No response
@mtreinish mtreinish added bug Something isn't working mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library labels Dec 7, 2023
@mtreinish mtreinish added this to the 1.0.0 milestone Dec 7, 2023
@jakelishman
Copy link
Member

This needs to be an error in practice; there's no way to communicate the creg / variable requirements of an Instruction's inner definition up to the main circuit, so allowing arbitrary control-flow within a semi-opaque instruction wouldn't work.

Totally agree that the error message should be way better.

@jakelishman
Copy link
Member

Really, circuit_to_instruction shouldn't even allow c_if in it - in practice, that hasn't ever worked fully correctly when registers are involved.

@maheshwaripranav
Copy link

maheshwaripranav commented Jan 30, 2024 •
edited
Loading

@jakelishman This used to work well until version 0.45.0
Could you suggest what alternative should be used for this as I am writing programs to implement error correction and if_test functionality is much used in my program?

@jakelishman
Copy link
Member

There should be no need to use circuit_to_instruction. If you want to build up circuit parts and then combine them into a larger circuit, you can use QuantumCircuit.compose to "inline" the smaller circuit into the bigger one. QuantumCircuit.append cannot really cope with complex classical data like control-flow constructs, because the necessary classical data available properly in the QuantumCircuit resource tracking through CircuitInstruction.

@jakelishman
Copy link
Member

There's no particular guarantee that any classical handling was working correctly before if circuit_to_instruction was used (also via QuantumCircuit.append) - the circuit would silently not notice if the classical control flow was represented incorrectly.

@jakelishman jakelishman modified the milestones: 1.0.0, 1.1.0 Feb 1, 2024
@jakelishman
Copy link
Member

We didn't get this done for 1.0, so we'll have to revisit for 1.1 and see what's reasonable for us to do within the stability policy.

@1ucian0 1ucian0 changed the title Control flow instructions don't work with `circuit_to_instruction Control flow instructions don't work with circuit_to_instruction Mar 3, 2024
@jakelishman jakelishman modified the milestones: 1.1.0, 1.1.1 May 1, 2024
@jakelishman jakelishman modified the milestones: 1.1.1, 1.1.2 Jun 21, 2024
@jakelishman jakelishman modified the milestones: 1.1.2, 1.3.0 Aug 2, 2024
@melechlapson
Copy link
Contributor

@jakelishman @mtreinish I can work on this. I just need clarification if the fix would be to remove the c_if option altogether or add a check to make sure that it is not a control_flow operation and throw an Exception if it is

@raynelfss raynelfss modified the milestones: 1.3.0, 1.4.0, 1.3.1 Nov 6, 2024
@Cryoris Cryoris modified the milestones: 1.3.0, 1.3.1 Nov 29, 2024
@kevinhartman kevinhartman modified the milestones: 1.3.1, 1.3.2 Dec 11, 2024
gschurck added a commit to Alice-Bob-SW/felis that referenced this issue Dec 17, 2024
@gschurck
fix(samples): make notebooks compatible with Qiskit 1.3
2d38880 
The notebook "Tomography of quantum teleportation" had the issue
"Invalid clbits for condition x" at transpilation with Qiskit 1.3 during
 HighLevelSynthesis with c_if() conditions.
To merge the two circuits of the notebooks, QuantumCircuit.append() was
used. But in this Github issue it's recommended to use
QuantumCircuit.compose() instead :
Qiskit/qiskit#11379 (comment)
To make it work we had to change the size of the sub-circuit to be the
same at the other one (3 qubits and 3 classical bits).
Moving to QuantumCircuit.compose() fixed the error with the conditions.

Refs: #8785
gschurck added a commit to Alice-Bob-SW/felis that referenced this issue Dec 19, 2024
@gschurck
fix(samples): make notebooks compatible with Qiskit 1.3
1b66471 
The notebook "Tomography of quantum teleportation" had the issue
"Invalid clbits for condition x" at transpilation with Qiskit 1.3 during
 HighLevelSynthesis with c_if() conditions.
To merge the two circuits of the notebooks, QuantumCircuit.append() was
used. But in this Github issue it's recommended to use
QuantumCircuit.compose() instead :
Qiskit/qiskit#11379 (comment)
To make it work we had to change the size of the sub-circuit to be the
same at the other one (3 qubits and 3 classical bits).
Moving to QuantumCircuit.compose() fixed the error with the conditions.

Refs: #8785
gschurck added a commit to Alice-Bob-SW/felis that referenced this issue Dec 20, 2024
@gschurck
fix(samples): make notebooks compatible with Qiskit 1.3
79915c7 
The notebook "Tomography of quantum teleportation" had the issue
"Invalid clbits for condition x" at transpilation with Qiskit 1.3 during
 HighLevelSynthesis with c_if() conditions.
To merge the two circuits of the notebooks, QuantumCircuit.append() was
used. But in this Github issue it's recommended to use
QuantumCircuit.compose() instead :
Qiskit/qiskit#11379 (comment)
To make it work we had to change the size of the sub-circuit to be the
same at the other one (3 qubits and 3 classical bits).
Moving to QuantumCircuit.compose() fixed the error with the conditions.

Refs: #8785
@ShellyGarion ShellyGarion modified the milestones: 1.3.2, 2.0.0 Jan 13, 2025
@1ucian0 1ucian0 linked a pull request Feb 24, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@1ucian0 1ucian0

Labels
bug Something isn't working mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library
Projects
None yet
Milestone
2.0.0
9 participants
@1ucian0 @kevinhartman @mtreinish @jakelishman @Cryoris @ShellyGarion @melechlapson @maheshwaripranav @raynelfss