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

Add Circuit.has_implicit_wireswaps property #1772

Merged
merged 6 commits into from
Feb 10, 2025

Conversation

jpacold
Copy link
Contributor

@jpacold jpacold commented Feb 10, 2025

Description

  • Added a readonly property bound to Circuit::has_implicit_wireswaps()
  • Added a unit test
  • Updated some existing tests -- replaced calls to Circuit.implicit_qubit_permutation() if the test only used the result to check for non-trivial permutations

Related issues

Closes #1665.

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.

@cqc-alec cqc-alec self-requested a review February 10, 2025 08:44
Copy link
Collaborator

@cqc-alec cqc-alec left a comment

Choose a reason for hiding this comment

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

Thanks! Minor comment on the changelog, and I'm not sure why we're seeing the warning that predicates_test.py line 1151 is unreachable?

@@ -9,6 +9,7 @@ Features:
* Add Python 3.13 support to the ZX module.
* Add :py:meth:`QubitPauliOperator.get_dict` method.
* Add :py:meth:`Circuit.add_circuit_with_map` method.
* Add :py:attr:`Circuit.has_implicit_wireswaps` property.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be under a new "Unreleased" heading above please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

assert c.has_implicit_wireswaps

# Property should be read-only
with pytest.raises(AttributeError):
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems that a linter is complaining that this line is unreachable; I don't really see why...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems to be a known mypy issue. I added an ignore comment; mypy now says the file is OK on my machine but I am not 100% sure this will fix the CI.

@cqc-alec
Copy link
Collaborator

Made #1774 to silence the ruff warning. (Only the latest version of ruff detected this.)

@cqc-alec cqc-alec merged commit 453b7bf into CQCL:main Feb 10, 2025
30 checks passed
@jpacold jpacold deleted the has_implicit_permutation branch February 10, 2025 23:32
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.

Add Circuit.has_implicit_permutation
2 participants