-
Notifications
You must be signed in to change notification settings - Fork 59
feat: add cluster visualization for WhileOp, IfOp, ForOp
#2234
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
feat: add cluster visualization for WhileOp, IfOp, ForOp
#2234
Conversation
This reverts commit 59e7826.
…cuit_dag.py Co-authored-by: Mudit Pandey <[email protected]>
mudit2812
left a comment
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.
2 blocking comments:
if-elif-elseblocks seems to be incorrectly handledWhileOpshould probably have a main cluster with 2 clusters inside it, similar to anIfOp, for thebefore/after_regions
frontend/catalyst/python_interface/visualization/construct_circuit_dag.py
Outdated
Show resolved
Hide resolved
frontend/catalyst/python_interface/visualization/construct_circuit_dag.py
Outdated
Show resolved
Hide resolved
frontend/catalyst/python_interface/visualization/construct_circuit_dag.py
Show resolved
Hide resolved
frontend/catalyst/python_interface/visualization/construct_circuit_dag.py
Show resolved
Hide resolved
frontend/catalyst/python_interface/visualization/construct_circuit_dag.py
Outdated
Show resolved
Hide resolved
frontend/catalyst/python_interface/visualization/construct_circuit_dag.py
Outdated
Show resolved
Hide resolved
frontend/test/pytest/python_interface/visualization/test_construct_circuit_dag.py
Outdated
Show resolved
Hide resolved
…cuit_dag.py Co-authored-by: Mudit Pandey <[email protected]>
mehrdad2m
left a comment
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.
Thanks @andrijapau, looks good. I only have some concerns regarding the flattening logic.
frontend/test/pytest/python_interface/visualization/test_construct_circuit_dag.py
Show resolved
Hide resolved
frontend/catalyst/python_interface/visualization/construct_circuit_dag.py
Outdated
Show resolved
Hide resolved
frontend/catalyst/python_interface/visualization/construct_circuit_dag.py
Outdated
Show resolved
Hide resolved
…ruct_circuit_dag.py Co-authored-by: Mehrdad Malek <[email protected]>
mehrdad2m
left a comment
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 will approve to not block the PR, I think I looks good as long as the unresolved conversation is fixed.
Thanks, I think the fix is already in place. I posted in slack here to see product's opinion. |
This PR adds basic cluster visualization for these control flow operators.
[sc-103462]
[sc-103464]
[sc-103465]