Skip to content

Conversation

LEXUGE
Copy link
Collaborator

@LEXUGE LEXUGE commented Aug 29, 2025

Created a new GraphSim module.

  • The code is properly formatted and commented.
  • Substantial new functionality is documented within the docs.
  • All new functionality is tested.
  • All of the automated tests on github pass.
  • We recently started enforcing formatting checks. If formatting issues are reported in the new code you have written, please correct them. There will be plenty of old code that is flagged as we are slowly transitioning to enforced formatting. Please do not worry about or address older formatting issues -- keep your PR just focused on your planned contribution.

@LEXUGE LEXUGE requested review from Krastanov and Copilot August 29, 2025 10:00
Copy link

@Copilot Copilot AI left a 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 introduces a new GraphSim module to organize graph state simulation functionality. The change wraps existing graph simulation code in a dedicated module and updates import statements throughout the codebase to use the new module structure.

  • Wraps existing graph state functionality in a new GraphSim module
  • Updates all test files to import from QuantumClifford.GraphSim instead of QuantumClifford
  • Imports specific functions from the new module into the main QuantumClifford namespace

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/graphs/graphs.jl Creates the new GraphSim module wrapper with necessary imports
src/QuantumClifford.jl Imports key GraphSim functions into main namespace
test/test_graphs_tables.jl Updates imports to use GraphSim module
test/test_graphs_single_qubit_gates.jl Updates imports to use GraphSim module
test/test_graphs_cphase.jl Updates imports to use GraphSim module
test/test_graphs.jl Updates imports to use GraphSim module

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

previously it relies on the side effect that graphs.jl imports Graphs.
@LEXUGE LEXUGE force-pushed the graph_sim_module branch 2 times, most recently from c461c36 to 5d4cabd Compare August 29, 2025 20:32
@LEXUGE
Copy link
Collaborator Author

LEXUGE commented Aug 29, 2025

hmmmm @Krastanov I am not sure how to fix the doctest. Supposedly if it uses QuantumClifford then all the functions should be exported already.

Copy link
Member

@Krastanov Krastanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the doctests have not finished running, so I am not sure about them.

As for the documentation itself, the failure is in resolving the references. The rules for them are a bit different. Check the various module definitions in docs/make.jl and just add an extra one the way there are a few already for other submodules.

@LEXUGE LEXUGE force-pushed the graph_sim_module branch 4 times, most recently from f055ba5 to 63fd922 Compare August 30, 2025 21:07
- @ref needs to reference to some docstring already rendered in the menu
@LEXUGE
Copy link
Collaborator Author

LEXUGE commented Aug 30, 2025

@Krastanov I think it's now working:

  • public API is exposed directly under QuantumClifford, not breaking existing API
  • However, as benefits of the module, internal symbols won't be exposed / cause side-effects on other parts of the code.

@LEXUGE LEXUGE requested a review from Krastanov August 31, 2025 09:54
@LEXUGE LEXUGE mentioned this pull request Aug 31, 2025
4 tasks
@Krastanov
Copy link
Member

@LEXUGE , could you remind me what the status here is? Is this ready for merge after a review or do we have any outstanding questions about it?

@LEXUGE
Copy link
Collaborator Author

LEXUGE commented Sep 16, 2025

It doesn't have anything outstanding pending on me. So should be OK to merge after a review

@LEXUGE
Copy link
Collaborator Author

LEXUGE commented Sep 30, 2025

@Krastanov Would you like to review this PR so I could push forward the graph state measurement and remaining cleanup/documentation.

Thanks!

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