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

Mismatched serialisation of Consts between rust and python #1424

Closed
aborgna-q opened this issue Aug 13, 2024 · 0 comments · Fixed by #1427
Closed

Mismatched serialisation of Consts between rust and python #1424

aborgna-q opened this issue Aug 13, 2024 · 0 comments · Fixed by #1427
Assignees
Labels
bug Something isn't working

Comments

@aborgna-q
Copy link
Collaborator

This is the serialised repr of a hugr_core::std_extensions::arithmetic::float_types::ConstF64

{
    "parent": 1,
    "op": "Const",
    "v": {
        "v": "Extension",
        "typ": {
            "t": "Opaque",
            "extension": "arithmetic.float.types",
            "id": "float64",
            "args": [],
            "bound": "C"
        },
        "value": {
            "c": "ConstF64",
            "v": {
                "value": 0.5
            }
        },
        "extensions": [
            "arithmetic.float.types"
        ]
    }
}

Compare it to the encoding of a hugr.std.float.FloatVal for the same value

{
    "parent": 0,
    "op": "Const",
    "v": {
        "v": "Extension",
        "extensions": [],
        "typ": {
            "t": "Opaque",
            "extension": "arithmetic.float.types",
            "id": "float64",
            "args": [],
            "bound": "C"
        },
        "value": {
            "c": "float",
            "v": 0.5
        }
    }
},

Note:

  • The Cost op missing its extensions
  • The value encoded directly as a Value instead of a dict

Code used to generate the examples:

Rust 🦀
fn example_float_const() {
    let hugr = build_main(
        Signature::new(type_row![], type_row![FLOAT64_TYPE])
            .with_extension_delta(float_types::EXTENSION_ID)
            .into(),
        |mut f_build| {
            let float_val = f_build.add_load_const(Value::from(ConstF64::new(0.5)));
            f_build.finish_with_outputs([float_val])
        },
    )
    .unwrap();

    println!("{}", serde_json::to_string_pretty(&hugr).unwrap());
}
Python 🐍
def test_float_const():
    h = Dfg()
    val = h.load(FloatVal(0.5))
    h.set_outputs(val)

    print(h.hugr.to_json())
@aborgna-q aborgna-q added the bug Something isn't working label Aug 13, 2024
@aborgna-q aborgna-q self-assigned this Aug 14, 2024
github-merge-queue bot pushed a commit that referenced this issue Aug 14, 2024
Fixes #1424 

I updated the roundtrip validation to compare decoded jsons rather than
strings.

The values were manually compared against a rust serialization output.
We cannot do better without python-side extension loading.
github-merge-queue bot pushed a commit to CQCL/guppylang that referenced this issue Aug 21, 2024
This is an in-progress transplant of guppy from using an ad-hoc `Hugr`
definition into the builder from the `hugr` library.

The main mismatch between the old and new builders is that guppy used to
lazily add typed ports to the graph nodes as it needed. Since the new
builder requires operations to define their signature, we end up having
to edit the code everywhere from `guppylang.compiler` to the
`guppylang.prelude` definitions.

My first goal is to get this working, and then I will try and split some
changes from this PR (although most of it will have to be merged
monolithically).
 
Current test status:
- 390 $\color{green}\text{passed}$
- 14 $\color{yellow}\text{skipped}$
- 0 $\color{red}\text{failed}$

These issues currently breake some tests. I'll fix them before merging
the PR
- CQCL/hugr#1319
  ~~Required to store the module names in the hugr metadata~~
- CQCL/hugr#1424
  ~~Required for pytket and llvm integration~~
- ~~Release hugr and tket2 with the latest changes~~

Closes #257. Closes #85

BREAKING CHANGE: Removed `guppylang.hugr_builder.hugr.Hugr`, compiling a
module returns a `hugr.Hugr` instead.

---------

Co-authored-by: Mark Koch <48097969+mark-koch@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant