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

Long proofs unable to load #229

Open
Aerylia opened this issue Jan 25, 2024 · 17 comments
Open

Long proofs unable to load #229

Aerylia opened this issue Jan 25, 2024 · 17 comments
Labels
bug Something is not right Proof mode Issues and enhancements related to Proof mode

Comments

@Aerylia
Copy link
Collaborator

Aerylia commented Jan 25, 2024

I have created a bunch of long proofs and most of them are unable to load.
I get this message:
image
The number changes for different files.

No errors from the terminal.

I am on Macbook Air M1, MacOS Sonoma 14.2.1

@RazinShaikh RazinShaikh added bug Something is not right Proof mode Issues and enhancements related to Proof mode labels Jan 25, 2024
@RazinShaikh
Copy link
Collaborator

Hi @Aerylia, I just merged the pull request #213 by @wlcsm which changes the file format of the proof file. I believe that should fix the issue. Could you please try and check if it works for you now? Note that the file format is not backward compatible so you need to save your proofs again.

@Aerylia
Copy link
Collaborator Author

Aerylia commented Jan 25, 2024

Hi, I am now getting a different error:

image

@RazinShaikh
Copy link
Collaborator

have you saved the proof again or is this an old file? Can you attach the proof file here?

@RazinShaikh RazinShaikh reopened this Jan 25, 2024
@Aerylia
Copy link
Collaborator Author

Aerylia commented Jan 25, 2024

It is the old file. I don't have the proof open. I saved it to update my OS.
The problem is that I would like to work on the proof, but I cannot load it anymore. If it was open, there would be no problem.

Below the file. I have changed the extension to .txt so that Github doesn't complain.
QAOA derivation1.txt

@Aerylia
Copy link
Collaborator Author

Aerylia commented Jan 25, 2024

But if you can tell me which version of zxlive I need to open this file, I'll pull an older version. I needed these diagrams tonight...

@RazinShaikh
Copy link
Collaborator

ok let me try to debug and see if we can recover the proof from that file

@dlyongemallo
Copy link
Contributor

Isn't this just issue #207 (which was caused by PR #204)? I think PR #204 should be reverted. It's supposed to fix saving proofs with parametrized spiders, but it makes it so that proofs can't be loaded at all (as the vertices are erroneously mislabeled after the first proof step), which is arguably worse.

As long as the proofs internally save the graphs as JSON rather than Tikz format, the saving/loading bug will happen.

@dlyongemallo
Copy link
Contributor

I guess ZXLive is pre-release or beta software so there's no guarantee that file formats won't change, but it would still be good if the changes are better documented and users given more warning about such changes. In PR #222, I added a test which makes it so that any PR which is backwards incompatible have to explicitly update the test.

I think in general any PR which breaks backwards-compatibility should say so very clearly in the commit's message and PR description. In some other projects I've worked on, for example, there's a policy that the commit message for such changes should be, e.g., "[BREAKING CHANGE] save proofs as JSON rather than Tikz format", or something like that. Then it's clear to anyone looking through the commits to understand why something they expected to work isn't working any more.

@Aerylia
Copy link
Collaborator Author

Aerylia commented Jan 30, 2024

As additional information: It's not about a change of file formats. I used the same version to save as I used to load and they were incompatible.
Additionally, I was unable to add parameterized gates to my diagrams because they were not a fraction of pi.

@RazinShaikh
Copy link
Collaborator

Does this happen on the latest version?

@dlyongemallo
Copy link
Contributor

In particular, please make sure you have the latest version of both ZXLive and PyZX. In your zxlive directory/virtual environment, run pip install -r requirements.txt to update to the latest changes in pyzx on GitHub.

@wlcsm
Copy link

wlcsm commented Jan 31, 2024

I guess ZXLive is pre-release or beta software so there's no guarantee that file formats won't change, but it would still be good if the changes are better documented and users given more warning about such changes. In PR #222, I added a test which makes it so that any PR which is backwards incompatible have to explicitly update the test.

I think in general any PR which breaks backwards-compatibility should say so very clearly in the commit's message and PR description. In some other projects I've worked on, for example, there's a policy that the commit message for such changes should be, e.g., "[BREAKING CHANGE] save proofs as JSON rather than Tikz format", or something like that. Then it's clear to anyone looking through the commits to understand why something they expected to work isn't working any more.

I agree that we should better label backwards compatible changes. One way we could encourage this would be to use an PR template that asks whether the changes are backward compatible with a reminder to label them as so in the commit message. The reviewers can also verify the commit messages are appropriately labelled.

The test you added is extremely useful!
I would also like to suggest we add a version field into the proofs containing the ZXLive version that was used to save the proof/diagram to aid with debugging. It could also be used to inform the user which version to switch to in the case of an error which will speed up resolution time for users who aren't able to inspect the git history.

@wlcsm
Copy link

wlcsm commented Jan 31, 2024

Isn't this just issue #207 (which was caused by PR #204)? I think PR #204 should be reverted. It's supposed to fix saving proofs with parametrized spiders, but it makes it so that proofs can't be loaded at all (as the vertices are erroneously mislabeled after the first proof step), which is arguably worse.

As long as the proofs internally save the graphs as JSON rather than Tikz format, the saving/loading bug will happen.

I'm in favour of reverting the change as well. JSON serialisation is pyxz doesn't preserve the ordering of vertices and I currently have no quick fix for it. Though it is something I intend of resolving in the short term, testing the change may prove to take up the most time

@wlcsm
Copy link

wlcsm commented Jan 31, 2024

@Aerylia Could you provide a screenshot of the proof or a simplified example? Since the file you provided can't be loaded, I don't know how to replicate the bug.

@wlcsm
Copy link

wlcsm commented Jan 31, 2024

Additionally, I was unable to add parameterized gates to my diagrams because they were not a fraction of pi.

We currently don't support this. Our logic for parameterised gates is very rudimentary. I have recorded this feature request in #232

@RazinShaikh
Copy link
Collaborator

I think we already support this though. If it doesn't work, then it must be some sort of bug

@dlyongemallo
Copy link
Contributor

Additionally, I was unable to add parameterized gates to my diagrams because they were not a fraction of pi.

Can you clarify this? Do you mean that you want the parameter to be some value a, whose value is, e.g., 0.5, and you actually want the spider to have a phase of 0.5 and not 0.5 * pi?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not right Proof mode Issues and enhancements related to Proof mode
Projects
None yet
Development

No branches or pull requests

4 participants