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.

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

Remove all references to dead vertices in ZX extraction #1690

Merged
merged 3 commits into from
Nov 21, 2024

Conversation

willsimmons1465
Copy link
Contributor

@willsimmons1465 willsimmons1465 commented Nov 21, 2024

Description

During extraction, vertices are gradually removed from a ZX graph as we extract gates. Some additional tracking info is maintained via sets and maps of vertices. When vertices are removed, this shouldn't cause issues with looking up remaining vertices in these sets and maps.

HOWEVER, if needed it will perform an input extension which adds new vertices to the graph. Depending on how boost is feeling with memory management, it may reuse the memory locations of removed vertices for these new ones. Because the vertex descriptors are void* pointers under the hood, this clash of memory locations means we can get false positives when looking up vertices in the sets/maps.

#1566 and a related circuit found internally were shown to hit errors during extraction which at first seemed device-dependent, but was found to just be non-deterministic on any device. These were due to one of these false positive lookups which cause some vertices to be ignored when looking for a flow (hence the error message Error during extraction from ZX diagram: diagram does not have gflow).

Given the bug only occurs with some probability (that may be device-dependent), the tests for this fix run the circuits a bunch of times to reduce the likelihood of a false positive for CI.

Related issues

Closes #1566

Checklist

  • I have performed a self-review of my code.
  • I have commented hard-to-understand parts of my code.
  • I have made corresponding changes to the public API documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the changelog with any user-facing changes.

Copy link
Contributor

@sjdilkes sjdilkes left a comment

Choose a reason for hiding this comment

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

LGTM! Except I don't think we need to loop the tests in test_ZXSimp 50 times - if you've confirmed it manually locally then I'm happy to assume it's all fixed and just run each case once.

@willsimmons1465 willsimmons1465 merged commit 88c0481 into main Nov 21, 2024
32 checks passed
@willsimmons1465 willsimmons1465 deleted the bugfix/zxbug branch November 21, 2024 16:04
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.

ZXGraphlikeOptimisation() can cause RunTime error
2 participants