Skip to content

Implement JSON serialization for PauliSum #3071

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

Closed
kevinsung opened this issue Jun 4, 2020 · 5 comments · Fixed by #5367
Closed

Implement JSON serialization for PauliSum #3071

kevinsung opened this issue Jun 4, 2020 · 5 comments · Fixed by #5367
Assignees
Labels
area/paulis area/serialization kind/feature-request Describes new functionality status/needs-agreed-design We want to do this, but it needs an agreed upon design before implementation triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@kevinsung
Copy link
Collaborator

No description provided.

@kevinsung kevinsung changed the title Define JSON serialization for PauliSum Implement JSON serialization for PauliSum Jun 4, 2020
@dabacon dabacon self-assigned this Jun 4, 2020
@dabacon
Copy link
Collaborator

dabacon commented Jun 9, 2020

@mpharrigan this is going to require serializing frozenset. Do you have any objections to this? I'm going to do it in a way that it preserves roundtrip (there is a crazy cool hack you could do which is to pass in the values of the set as keys in a dict with all values None, but figure that's not a good practice :) )

@mpharrigan
Copy link
Collaborator

Do you mean adding generic support for frozensets in the cirq encoder? Or custom logic in PauliSum for turning the keys from frozenset to lists and then back to frozenset in PauliSum's deserializer?

If the latter, that's expected although the plumbing is a little intricate in this particular case.

If the former, how do you intend to tag the resulting JSON so it's known to be a frozenset during deserialization?

@dabacon
Copy link
Collaborator

dabacon commented Jun 9, 2020

Oh I was thinking the former, but you are correct that could be a way to handle it. Was just going to re-use the "cirq_type" with frozenset. An alternative would be to create a different key "builtin-type". I think there was some discussion of this problem over in the numpy array serialization discussion.

Now that I think about it maybe your first suggestion is the better approach.

@mpharrigan
Copy link
Collaborator

Let me know how it goes. I'm not opposed to adding support for more basic containers in principle. I think since _UnitPauliString is an implementation detail it's not strictly necessary for this use case

@dabacon
Copy link
Collaborator

dabacon commented Jun 12, 2020

I wanted to see what it looked like. One issue you run into is tuples, which are default serialized to lists. You can fix this https://stackoverflow.com/questions/15721363/preserve-python-tuples-with-json but not really a great hack.

@balopat balopat added this to the 0.9.0 milestone Jul 28, 2020
@balopat balopat added area/paulis area/serialization kind/feature-request Describes new functionality triage/needs-feasibility [Feature requests] Needs design work to prove feasibility before accepting triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on status/needs-agreed-design We want to do this, but it needs an agreed upon design before implementation and removed triage/needs-feasibility [Feature requests] Needs design work to prove feasibility before accepting labels Aug 27, 2020
@balopat balopat removed this from the 0.9.0 milestone Sep 16, 2020
CirqBot pushed a commit that referenced this issue May 16, 2022
Fixes #3071 

One approach to this would have been to serialize frozenset and tuple in Cirq, but this instead takes the approach of not relying on this but appropriately serializing and deserializing these.
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
Fixes quantumlib#3071 

One approach to this would have been to serialize frozenset and tuple in Cirq, but this instead takes the approach of not relying on this but appropriately serializing and deserializing these.
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
Fixes quantumlib#3071 

One approach to this would have been to serialize frozenset and tuple in Cirq, but this instead takes the approach of not relying on this but appropriately serializing and deserializing these.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/paulis area/serialization kind/feature-request Describes new functionality status/needs-agreed-design We want to do this, but it needs an agreed upon design before implementation triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants