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

selecting spider with Poly phase results in NotImplementedError in proof mode #234

Open
dlyongemallo opened this issue Feb 8, 2024 · 3 comments
Labels
Priority: Medium Type: bug Something is not right

Comments

@dlyongemallo
Copy link
Contributor

Steps to repro:

  1. Start a new graph (or edit the demo graph)
  2. Add a Z-spider with a phase of, e.g., a (or edit any spider of the demo graph to give it a Poly phase)
  3. Click "Start derivation"
  4. Select the spider (either by clicking on it, or dragging a selection box that includes it)

Expected: spider is selected

Actual: NotImplementedError is raised with message "denominator not implemented for symbolic Poly".

@RazinShaikh
Copy link
Collaborator

When PyZX was built, phases were fractions multiplied by pi. That's why a lot of functions explicitly ask for numerator or denominator of phases. It is not clear to me what should be returned to these functions when they ask for numerator/denominator of symbolic variables. So I left these functions unimplemented. If you have any suggestions on how to tackle this, please let me know.
https://github.com/Quantomatic/pyzx/blob/346346159b53221617c89193158a050280c4e4a9/pyzx/symbolic.py#L311-L317

@dlyongemallo
Copy link
Contributor Author

I think at this point the right thing to do would be to write a design doc for PyZX and carefully think through of its components and how they should handle phases specified in various ways, and then go through the code to ensure that everything behaves as intended or even rewrite some of the components from scratch where it makes sense. I think any other approach is likely to lead to serious tech debt and difficult-to-reason-about bugs later.

Sorry if that isn't immediately helpful.

@lia-approves
Copy link
Collaborator

@hm919 and I fixed this bug by submitting PR zxcalc/pyzx#234 to PyZX
However, we agree a design doc for PyZX would be better and reached out to the PyZX maintainers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Medium Type: bug Something is not right
Projects
None yet
Development

No branches or pull requests

4 participants