Skip to content

Fix classical control rendering for gate #278

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

gadhvirushiraj
Copy link
Contributor

  • Fixed rendering issue for classical control on gates for both renderers.
  • Optimize: Removed _draw_singleq_gate for rendering in MatRenderer.

@BoxiLi
Copy link
Member

BoxiLi commented Jun 15, 2025

Thanks a lot for such a quick PR!

I noticed two things:

  1. This classical control line should also be added when we compute the layer of the gate.
from qutip_qip.circuit import QubitCircuit
qc = QubitCircuit(2, num_cbits=2)
qc.add_gate("X", targets=0, classical_controls=0, classical_control_value=0)
qc.add_gate("X", targets=1, classical_controls=1, classical_control_value=1)
qc.draw()

image

  1. I think there might be a misunderstanding of the classical_controls and classical_control_value. I didn't realize it when you asked that question on Discord. I think here we should read classical_controls, which should be a list that records the control classical bits. And then we draw a circuit on it, like in the quantum controlled gate

image

@gadhvirushiraj
Copy link
Contributor Author

gadhvirushiraj commented Jun 21, 2025

  1. This classical control line should also be added when we compute the layer of the gate.

We dont maintain a record for classical lines. But we just augment this by maintaining gaps in qlines. But it should be now fixed for MatRenderer.

I have modified it to read classical_controls too.

Edit: TextRenderer is yet to fix

@BoxiLi
Copy link
Member

BoxiLi commented Jun 22, 2025

Great! Looks good!

@BoxiLi
Copy link
Member

BoxiLi commented Jul 9, 2025

Looks good! Rendered nicely on my machine. Would be nice to add one test case for the text renderer. Just to be safe.

The failing test is unrelated, but the one on matplotlib seems to be a warning from the old matplotlib 3.9

@BoxiLi
Copy link
Member

BoxiLi commented Aug 7, 2025

Thanks! This looks good to me. Shall I merge? @gadhvirushiraj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants