Skip to content

Conversation

RazinShaikh
Copy link
Contributor

Please merge after #354

This PR modifies scalar.py to support symbolic phases. As such, it removes any explicit calculation of the complex scalar outside the dedicated to_number method.

The main change is due to the add_spider_pair method. The scalar added by this method is 1 + e^(i pi p1) + e^(i pi p2) - e^(i pi (p1+p2)) where p1 and p2 are phases of the two spiders. In the case when p1 and p2 are both non-clifford, there is no way of writing this scalar using scalar.phase and scalar.phase_node. So we introduce a new variable scalar.sum_of_phases which represents the term (c1*exp(i*phase1) + ... + cn*exp(i*phaseN)) as a dictionary mapping phase->coeffecient. Rest of the changes are in support of adding the sum_of_phases variable.

@jvdwetering
Copy link
Collaborator

These are some pretty complicated changes. Would it make sense to add a bunch of tests to make sure things work correctly? Also including all the various ways that the phase should be translated back into a string or saved to json?

Merge pull request zxcalc#354 from RazinShaikh/symbolic-lc-pivot
@RazinShaikh
Copy link
Contributor Author

Sorry, it's less complicated than it looks. The branch was split from the branch that fixed local complementation, etc so it had all the changes from that branch shown here again. Now I have merged the master branch onto this PR and the "files changed" went from 9 to 2.

@RazinShaikh
Copy link
Contributor Author

But I am happy to add some tests for the changes in this PR

@jvdwetering
Copy link
Collaborator

Yes, this looks simpler now.
Btw, maybe not in this PR, but: is there some way in which we can instantiate a parameter in a diagram, including in the scalar? Like if I have some parameter alpha, and I then decide this is equal to pi/4, is there a way to make this happen on the graph and scalar?

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