-
Notifications
You must be signed in to change notification settings - Fork 9
Refactoring ZX implementation #114
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
Conversation
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.
Pull Request Overview
This PR refactors the ZXCalculus package to improve code organization and maintainability by:
- Splitting monolithic files into modular interface and implementation structures
- Reorganizing test files to match the new source structure
- Deprecating
ZXDiagramin favor ofZXCircuitfor circuit operations - Introducing abstract interfaces for ZX-diagrams and circuits
Key Changes:
- Split large files (
zx_diagram.jl,zx_graph.jl,rules.jl) into smaller, focused modules - Created interface definitions for
AbstractZXDiagramandAbstractZXCircuit - Reorganized tests into module-scoped testsets with proper nesting
- Added deprecation warnings for
ZXDiagramandconvert_to_zxd
Reviewed Changes
Copilot reviewed 110 out of 114 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/runtests.jl | Restructured test organization with module scoping and new testset hierarchy |
| test/ZX/zx_layout.jl | New test file for ZXLayout type |
| test/ZX/rules/*.jl | Split rule tests into individual files by rule type |
| test/ZX/interfaces/*.jl | New interface tests for abstract types |
| test/ZX/implementations/*.jl | New implementation tests for concrete types |
| src/ZX/types/*.jl | Extracted type definitions (SpiderType, EdgeType, ZXLayout) |
| src/ZX/interfaces/*.jl | New interface definitions for abstract types |
| src/ZX/implementations/*.jl | Extracted implementations for ZXGraph, ZXDiagram, ZXCircuit |
| src/ZX/rules/*.jl | Split rules into individual files with named rule types |
| src/ZX/ir.jl | Refactored IR conversion with deprecation warnings |
| src/Utils/conversion.jl | Extracted conversion utilities from zx_diagram.jl |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Roger-luo @GiggleLiu Sorry for the gigantic PR. I believe it may be too much for a human to review all at once. I am not sure if it can be decomposed into smaller ones. Please let me know what I should do to push forward this refactoring. |
|
I briefly checked the interface changes. I think it is a nice move. |
|
I don't have a strong opinion on the refactor, but I agree this is a good generalization of the old design. |
|
@GiggleLiu @Roger-luo Thanks for the comments. In this case, shall I merge the PR directly? |
This PR is breaking because it introduces a new design to support general ZX-diagram rewriting for potential applications beyond circuit simplification.
The previous design was specific to circuit simplification:
ZXDiagramwas intended to provide a circuit-level interface.ZXDiagramwould be converted into aZXGraph, as most rewriting rules are only defined on theZXGraph.ZXGraphwould implicitly assume a circuit structure including inputs and outputs, while also tracking the master diagram for phase teleportation.The previous design limited the application of this package. For instance, it was not possible to use this package for rewriting general ZX-diagrams when they are not from a circuit due to these design constraints.
This PR implements the following design changes:
ZXGraphis now the foundational graphical infrastructure to support arbitrary ZX-diagram rewriting.ZXCircuithas been introduced to provide additional circuit structures beyond theZXGraph, including inputs/outputs and phase tracking.ZXDiagramhas been retained to ensure compatibility with the old interface.