new: define (de)serializers for chip specs #27
Conversation
| #[serde(rename(deserialize = "1Q"))] | ||
| #[serde(rename(serialize = "1Q"))] |
There was a problem hiding this comment.
Style: throughout, why not rename these together?
| #[serde(rename(deserialize = "1Q"))] | |
| #[serde(rename(serialize = "1Q"))] | |
| #[serde(rename = "1Q"))] |
| } | ||
|
|
||
| #[derive(Debug, serde::Serialize, serde::Deserialize, PartialEq)] | ||
| pub struct Metadata {} |
There was a problem hiding this comment.
Should this include a flattened map of miscellaneous fields, so that it's round-trip-able?
https://serde.rs/attr-flatten.html#capture-additional-fields
| #[serde(rename(deserialize = "type"))] | ||
| #[serde(rename(serialize = "type"))] | ||
| ty: String, |
There was a problem hiding this comment.
Style: why not just make the field-name type?
| #[serde(deserialize_with = "deserialize_wildcard")] | ||
| #[serde(serialize_with = "serialize_wildcard")] | ||
| Wildcard, |
There was a problem hiding this comment.
Would Wildcard(monostate::MustBe!("_")) work here?
| #[serde(rename(deserialize = "1Q"))] | ||
| #[serde(rename(serialize = "1Q"))] |
There was a problem hiding this comment.
| #[serde(rename(deserialize = "1Q"))] | |
| #[serde(rename(serialize = "1Q"))] | |
| #[serde(rename = "1Q")] |
| #[serde(rename(deserialize = "2Q"))] | ||
| #[serde(rename(serialize = "2Q"))] |
There was a problem hiding this comment.
| #[serde(rename(deserialize = "2Q"))] | |
| #[serde(rename(serialize = "2Q"))] | |
| #[serde(rename = "2Q")] |
| #[derive(Debug, serde::Serialize, serde::Deserialize, PartialEq)] | ||
| pub struct Metadata {} |
There was a problem hiding this comment.
What is the purpose of this struct?
| #[serde(rename(serialize = "1Q"))] | ||
| #[serde(rename_all = "lowercase")] | ||
| #[serde(untagged)] | ||
| pub enum TwoQ { |
There was a problem hiding this comment.
| #[serde(rename(serialize = "1Q"))] | |
| #[serde(rename_all = "lowercase")] | |
| #[serde(untagged)] | |
| pub enum TwoQ { | |
| #[serde(rename(serialize = "2Q"))] | |
| #[serde(rename_all = "lowercase")] | |
| #[serde(untagged)] | |
| pub enum TwoQ { |
| #[serde(rename(deserialize = "type"))] | ||
| #[serde(rename(serialize = "type"))] |
There was a problem hiding this comment.
| #[serde(rename(deserialize = "type"))] | |
| #[serde(rename(serialize = "type"))] | |
| #[serde(rename = "type")] |
| #[serde(untagged)] | ||
| pub enum Gate { | ||
| Measure { | ||
| operator: monostate::MustBe!("MEASURE"), | ||
| qubit: Qubit, | ||
| target: Option<MeasurementTarget>, | ||
| duration: f64, | ||
| fidelity: f64, | ||
| }, | ||
| Quantum { | ||
| operator: String, | ||
| parameters: Vec<Parameter>, | ||
| arguments: Vec<Argument>, | ||
| duration: f64, | ||
| fidelity: f64, | ||
| }, | ||
| } |
There was a problem hiding this comment.
I think this does the same thing and does not require the extra crate.
| #[serde(untagged)] | |
| pub enum Gate { | |
| Measure { | |
| operator: monostate::MustBe!("MEASURE"), | |
| qubit: Qubit, | |
| target: Option<MeasurementTarget>, | |
| duration: f64, | |
| fidelity: f64, | |
| }, | |
| Quantum { | |
| operator: String, | |
| parameters: Vec<Parameter>, | |
| arguments: Vec<Argument>, | |
| duration: f64, | |
| fidelity: f64, | |
| }, | |
| } | |
| #[serde(tag = "operator")] | |
| pub enum Gate { | |
| #[serde(rename = "MEASURE")] | |
| Measure { | |
| qubit: Qubit, | |
| target: Option<MeasurementTarget>, | |
| duration: f64, | |
| fidelity: f64, | |
| }, | |
| #[serde(other)] | |
| Quantum { | |
| operator: String, | |
| parameters: Vec<Parameter>, | |
| arguments: Vec<Argument>, | |
| duration: f64, | |
| fidelity: f64, | |
| }, | |
| } |
| #[serde(deserialize_with = "deserialize_wildcard")] | ||
| #[serde(serialize_with = "serialize_wildcard")] |
There was a problem hiding this comment.
Would this do the same thing and be simpler?
| #[serde(deserialize_with = "deserialize_wildcard")] | |
| #[serde(serialize_with = "serialize_wildcard")] | |
| #[serde(rename = "_")] |
There was a problem hiding this comment.
(Same for all the other cases as well)
There was a problem hiding this comment.
Doesn't rename change the key name, not the value?
| use serde_json::json; | ||
|
|
||
| #[test] | ||
| fn deserialize_oneq() { |
| pub timestamp: Option<String>, | ||
| #[serde(deserialize_with = "deserialize_string_from_number")] | ||
| #[serde(serialize_with = "maybe_serialize_string_to_integer")] | ||
| pub version: String, |
There was a problem hiding this comment.
Seems like it might be better to have a Version struct instead of the manual serialization? Would also mean not needing the extra crate.
#[derive(Debug, serde::Serialize, serde::Deserialize)]
#[serde(untagged)]
pub enum Version {
Number(i32),
String(String),
}
| #[derive(Debug, serde::Serialize, serde::Deserialize, PartialEq)] | ||
| pub struct SpecsMap(HashMap<String, f64>); |
There was a problem hiding this comment.
Is there a specific reason for using a newtype pattern here instead of a type alias? Why not this?
| #[derive(Debug, serde::Serialize, serde::Deserialize, PartialEq)] | |
| pub struct SpecsMap(HashMap<String, f64>); | |
| pub type SpecsMap = HashMap<String, f64>; |
notmgsk
force-pushed
the
main
branch
2 times, most recently
from
9b564fa to
586eaeb
Compare
This MR adds
crate::chip::ChipSpec(and related types) which provide a Quilc-compatible definition of chip specifications.There is a corpus of data in
./test-dataused for parity testing (deserialize->serialize->compare with original). It has been copied from the Quilc repository with small edits to fix JSON syntax and to change the types of some data (e.g. replace"true"withtrue).