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

feat: Add serialization schema for metadata #1038

Merged
merged 2 commits into from
May 14, 2024
Merged

Conversation

doug-q
Copy link
Collaborator

@doug-q doug-q commented May 14, 2024

fixes #954.

We support deserializing HUGRs that do not have a metadata field.

We could optimise a little by not serializing a metadata field when all node metadata fields are null, but for now we don't bother.

import hugr

Port = tuple[NodeID, int | None] # (node, offset)
Edge = tuple[Port, Port]


class SerialHugr(BaseModel):
class SerialHugr(ConfiguredBaseModel):
Copy link
Collaborator Author

@doug-q doug-q May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very awkward that his slipped through. The consequence of this is that the "Strict Schema" testing has not been operating on the SerialHugr node(although it has on its children).

Once I fixed this, many tests failed due to an "extra" "metadata" field being serialised!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch. Good catch!

@doug-q doug-q requested a review from aborgna-q May 14, 2024 13:59
Copy link

codecov bot commented May 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.31%. Comparing base (10e3158) to head (ffdfb80).

❗ Current head ffdfb80 differs from pull request most recent head b103ca0. Consider uploading reports for the commit b103ca0 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1038   +/-   ##
=======================================
  Coverage   86.30%   86.31%           
=======================================
  Files          82       82           
  Lines       17235    17239    +4     
  Branches    17235    17239    +4     
=======================================
+ Hits        14875    14879    +4     
  Misses       1540     1540           
  Partials      820      820           
Flag Coverage Δ
rust 86.31% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@aborgna-q aborgna-q left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

import hugr

Port = tuple[NodeID, int | None] # (node, offset)
Edge = tuple[Port, Port]


class SerialHugr(BaseModel):
class SerialHugr(ConfiguredBaseModel):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch. Good catch!

hugr/src/hugr/serialize.rs Outdated Show resolved Hide resolved
Co-authored-by: Agustín Borgna <121866228+aborgna-q@users.noreply.github.com>
@doug-q doug-q enabled auto-merge May 14, 2024 15:27
@doug-q doug-q added this pull request to the merge queue May 14, 2024
Merged via the queue into main with commit 19bac62 May 14, 2024
16 checks passed
@doug-q doug-q deleted the doug/metadata-schema branch May 14, 2024 15:31
github-merge-queue bot pushed a commit that referenced this pull request May 20, 2024
## 🤖 New release
* `hugr`: 0.3.1 -> 0.4.0 (⚠️ API breaking changes)

### ⚠️ `hugr` breaking changes

```
--- failure inherent_method_const_removed: pub method is no longer const ---

Description:
A publicly-visible method or associated fn is no longer `const` and can no longer be used in a `const` context.
        ref: https://doc.rust-lang.org/reference/const_eval.html
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/inherent_method_const_removed.ron

Failed in:
  ConstF64::new in /tmp/.tmpwo5blB/hugr/hugr/src/std_extensions/arithmetic/float_types.rs:43

--- failure struct_missing: pub struct removed or renamed ---

Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/struct_missing.ron

Failed in:
  struct hugr::ops::constant::ExtensionValue, previously in file /tmp/.tmpq1W6bC/hugr/src/ops/constant.rs:184
```

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## 0.4.0 (2024-05-20)

### Bug Fixes

- Disallow non-finite values for `ConstF64`
([#1075](#1075))
- Serialization round-trips
([#948](#948))
- [**breaking**] Combine `ConstIntU` and `ConstIntS`
([#974](#974))
- Disable serialisation tests when miri is active
([#977](#977))
- [**breaking**] Serialisation schema
([#968](#968))
- Correct constant fold for `fne`.
([#995](#995))
- [**breaking**] Serialisation fixes
([#997](#997))
- [**breaking**] OpDef serialisation
([#1013](#1013))
- NaryLogicOp constant folding
([#1026](#1026))

### Features

- Add verification to constant folding
([#1030](#1030))
- Add `Const::get_custom_value`
([#1037](#1037))
- Add serialization schema for metadata
([#1038](#1038))
- 'Replace' rewrite returns node map
([#929](#929))
- `new` methods for leaf ops
([#940](#940))
- Add `string` type and `print` function to `prelude`
([#942](#942))
- `CustomOp::extension` utility function
([#951](#951))
- [**breaking**] Add `non_exhaustive` to various enums
([#952](#952))
- Encoder metadata in serialized hugr
([#955](#955))
- [**breaking**] Bring back Value
([#967](#967))
- Add LoadFunction node ([#947](#947))
- Add From impls for TypeArg
([#1002](#1002))
- Constant-folding of integer and logic operations
([#1009](#1009))
- [**breaking**] Update serialisation schema, implement `CustomConst`
serialisation ([#1005](#1005))
- Merge basic blocks algorithm
([#956](#956))
- [**breaking**] Allow panic operation to have any input and output
wires ([#1024](#1024))

### Refactor

- [**breaking**] Rename `crate::ops::constant::ExtensionValue` =>
`OpaqueValue` ([#1036](#1036))
- Outline hugr::serialize::test
([#976](#976))
- [**breaking**] Replace SmolStr identifiers with wrapper types.
([#959](#959))
- Separate extension validation from the rest
([#1011](#1011))
- Remove "trait TypeParametrised"
([#1019](#1019))

### Testing

- Reorg OutlineCfg/nest_cfgs tests so hugr doesn't depend on algorithm
([#1007](#1007))
- Ignore tests which depend on typetag when cfg(miri)
([#1051](#1051))
- Really ignore tests which depend on typetag when cfg(miri)
([#1058](#1058))
- Proptests for round trip serialisation of `Type`s and `Op`s.
([#981](#981))
- Add a test of instantiating an extension set
([#939](#939))
- Ignore serialisation tests when using miri
([#975](#975))
- [**breaking**] Test roundtrip serialisation against strict + lax
schema ([#982](#982))
- Fix some bad assert_matches
([#1006](#1006))
- Expand test of instantiating extension sets
([#1003](#1003))
- Fix unconnected ports in extension test
([#1010](#1010))

</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Douglas Wilson <douglas.wilson@quantinuum.com>
github-merge-queue bot pushed a commit that referenced this pull request May 20, 2024
🤖 I have created a release *beep* *boop*
---

##
[0.2.1](hugr-py-v0.2.0...hugr-py-v0.2.1)
(2024-05-20)

### ⚠ BREAKING CHANGES

* New serialisation schema
* rename `Const::const_type` and `Value::const_type` to
`Const::get_type` and `Value::get_type`. These now match several other
`get_type` functions ([#1005](#1005))
* Many uses of `Const` now use `Value`.

### Features

* Add serialization schema for metadata
([#1038](#1038))
([19bac62](19bac62))
* Add LoadFunction node
([#947](#947))
([81e9602](81e9602))
* Encoder metadata in serialized hugr
([#955](#955))
([0a44d48](0a44d48))
* Implement `CustomConst` serialization
([#1005](#1005))
([c45e6fc](c45e6fc))
* Revert the removal of `Value`
([#967](#967))
([0c354b6](0c354b6))
* Set default value for `Conditional.sum_rows`
([#934](#934))
([d69198e](d69198e))


### Bug Fixes

* `OpDef` serialization
([#1013](#1013))
([3d8f6f6](3d8f6f6))
* input_port_types and other helper functions on pydantic schema
([#958](#958))
([8651839](8651839))
* Remove insert_port_types for LoadFunction
([#993](#993))
([acca7bf](acca7bf))
* Serialisation for `Type`, `PolyFuncType`, and `Value`
([#968](#968))
([d913f40](d913f40))
* Serialization for `Op`s
([#997](#997))
([9ce6e49](9ce6e49))
* set `[build-system]` in `hugr-py/pyproject.toml`
([#1022](#1022))
([b9c3ee4](b9c3ee4))


### Code Refactoring

* rename `Const::const_type` and `Value::const_type` to
`Const::get_type` and `Value::get_type`. These now match several other
`get_type` functions ([#1005](#1005))
([c45e6fc](c45e6fc))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: Agustín Borgna <121866228+aborgna-q@users.noreply.github.com>
Co-authored-by: doug-q <141026920+doug-q@users.noreply.github.com>
Co-authored-by: Douglas Wilson <douglas.wilson@quantinuum.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metadata missing from the serialisation schema
2 participants