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

Serialisation roundtrip breaks LoadConst #779

Closed
aborgna-q opened this issue Jan 4, 2024 · 0 comments · Fixed by #783
Closed

Serialisation roundtrip breaks LoadConst #779

aborgna-q opened this issue Jan 4, 2024 · 0 comments · Fixed by #783
Assignees
Labels
bug Something isn't working
Milestone

Comments

@aborgna-q
Copy link
Collaborator

When running this simple test case that encodes/decodes a simple load const operation,

    #[test]
    fn constants() -> Result<(), Box<dyn std::error::Error>> {
        let mut builder = DFGBuilder::new(FunctionType::new(vec![], vec![FLOAT64_TYPE])).unwrap();
        let w = builder.add_load_const(ConstF64::new(0.5))?;
        let hugr = builder.finish_hugr_with_outputs([w], &FLOAT_OPS_REGISTRY)?;

        let ser = serde_json::to_string(&hugr)?;
        let deser = serde_json::from_str(&ser)?;
        assert_eq!(hugr, deser);
        Ok(())
    }

the extracted edge between the const definition and load gets an invalid target.

See the dot render:

Original Decoded
image image

This seems to be due to some inconsistent logic in the encoder/decoder, related to #777.

@aborgna-q aborgna-q added the bug Something isn't working label Jan 4, 2024
@aborgna-q aborgna-q self-assigned this Jan 4, 2024
@aborgna-q aborgna-q added this to the v0.1.0 milestone Jan 5, 2024
github-merge-queue bot pushed a commit that referenced this issue Jan 8, 2024
Closes #777.

Now `OpTrait` clearly separates `other_port` from `static_port`, and
`OpType` has a uniform interface for `value` ports (from the signature),
`static` ports (contstants), and `other` ports.

Fixes #779. The bug was caused by the encoder ignoring the constant
input port offset, so the decoder reconnected the edge to the
`StateOrder` port instead. Now we use the proper OpType methods.

Fixes #778.

---------

Co-authored-by: Seyon Sivarajah <seyon.sivarajah@quantinuum.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