feat!: HugrView API improvements #680

ss2165 · 2023-11-11T18:29:52Z

Closes Clean up OpType methods #495
Closes Refactor: move non_df_count out from OpValidityFlags #521
Closes feat: HugrView method for iterating over neighbouring ports #655
Closes Shorthand HugrView method for "connected port" #499
Closes Edge kind specific HugrView methods #506
Closes feat: HugrView Value port enhancements #653
OpType::signature returns option (non-dataflow ops don't return signature)
Implement try_into from OpType references in to inner references.

Doesn't necessarily do exactly as those issues specify - instead considers them holistically for a more unified interface. Easiest to review commit by commit.

Uses rust_version crate to use return position impl for the new HugrView methods that return iterators. This will be stable with 1.75 (which enters beta in a few days).

BREAKING_CHANGES: OpType and FunctionType methods renamed for clarity; OpType::signature returns Option<FuncType>.

codecov · 2023-11-11T18:35:02Z

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (c261bea) 83.93% compared to head (65ed37a) 84.26%.

Files	Patch %	Lines
src/ops.rs	92.30%	5 Missing ⚠️
src/types/signature.rs	83.33%	4 Missing ⚠️
src/builder/build_traits.rs	75.00%	1 Missing and 1 partial ⚠️
src/builder/module.rs	83.33%	0 Missing and 2 partials ⚠️
src/hugr/views.rs	97.40%	0 Missing and 2 partials ⚠️
src/ops/tag.rs	75.00%	2 Missing ⚠️
src/hugr/views/sibling_subgraph.rs	97.29%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #680      +/-   ##
==========================================
+ Coverage   83.93%   84.26%   +0.33%     
==========================================
  Files          66       66              
  Lines       12706    12961     +255     
  Branches    12706    12961     +255     
==========================================
+ Hits        10665    10922     +257     
+ Misses       1285     1282       -3     
- Partials      756      757       +1

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

Closes #521

Since it only applies to LoadConst and Call, use special case methods, and a tag to capture these ops.

And addds `static_input_port` method. Closes Clean up OpType methods #495

Uses return position impl because it will be stable soon and its much nicer. Closes #655

Closes Shorthand HugrView method for "connected port" #499

…eturn signature)

acl-cqc · 2023-11-14T12:19:27Z

src/hugr.rs

-        self.input_extensions
-            .as_ref()
-            .map(|rs| self.op.signature().with_input_extensions(rs.clone()))
+        self.input_extensions.as_ref().and_then(|rs| {


I kept pondering whether there's a "right" order to check these in but failed to come up with one. How about
if let (Some(rs), Some(sig)) = (self.input_extensions, self.op.signature()) {sig.with_input_extensions(rs.clone())} else {None}

I'm going to change this to keep this as previous behaviour (with an unwrap_or_default), I think changes to the inference logic should be a follow on

acl-cqc · 2023-11-14T12:21:29Z

src/hugr/rewrite/replace.rs

@@ -616,7 +616,7 @@ mod test {
        entry: bool,
    ) -> Result<BasicBlockID, BuildError> {
        let op: OpType = op.into();
-        let op_sig = op.signature();
+        let op_sig = op.signature().expect("dataflow op needs signature");


Hmmmm, we can't do something like impl Into<DataflowOpTrait> can we? Then we'd know we have a signature. I guess nothing to worry about in a test but generally it feels like it'd be good to try to know statically when we have an Op(Type) with a signature

done, I've used this logic in ~3 places, there are probably more. (Could be a follow up task for builder to require DataflowOpTrait in more places rather than Into<OpType>?)

ss2165 · 2023-11-14T15:00:18Z

Cargo.toml

@@ -50,6 +50,8 @@ petgraph = { version = "0.6.3", default-features = false }
 context-iterators = "0.2.0"
 serde_json = "1.0.97"
 delegate = "0.10.0"
+rustversion = "1.0.14"
+paste = "1.0"


I continue my brazen injecting of dtolnay crates

Note this one is already in dev-dependencies

aborgna-q

I'm happy with merging this, and iterating on other open issues as new PRs.

- Updates the API changes introduced in CQCL/hugr#680 - Uses the latest published portmatching instead of a git dependency - Moves the lint configuration to the workspace Cargo.toml (new to rust [1.74](https://blog.rust-lang.org/2023/11/16/Rust-1.74.0.html)) This throws warnings in older rust versions due to the unrecognised fields, but works alright.

## 🤖 New release * `quantinuum-hugr`: 0.1.0 <details><summary>Changelog</summary> <blockquote> ## 0.1.0 (2024-01-15) ### Bug Fixes - Subgraph boundaries with copies ([#440](#440)) - [**breaking**] Use internal tag for SumType enum serialisation ([#462](#462)) - Check kind before unwrap in insert_identity ([#475](#475)) - Allow for variables to get solved in inference ([#478](#478)) - In IdentityInsertion add noop to correct parent ([#477](#477)) - Failing release tests ([#485](#485)) - [**breaking**] Serialise `Value`, `PrimValue`, and `TypeArg` with internal tags ([#496](#496)) - Serialise custom constants with internal tag ([#502](#502)) - [**breaking**] Reduce max int width in arithmetic extension to 64 ([#504](#504)) - HugrView::get_function_type ([#507](#507)) - TODO in resolve_extension_ops: copy across input_extensions ([#510](#510)) - Use given input extensions in `define_function` ([#524](#524)) - Lessen requirements for hugrs in outline_cfg ([#528](#528)) - Make unification logic less strict ([#538](#538)) - Simple replace incorrectly copying metadata ([#545](#545)) - Account for self-referencial constraints ([#551](#551)) - Consider shunted metas when comparing equality ([#555](#555)) - Join labels in issue workflow ([#563](#563)) - Comment out broken priority code ([#562](#562)) - Handling of issues with no priority label ([#573](#573)) - Don't insert temporary wires when extracting a subgraph ([#582](#582)) - Improve convexity checking and fix test ([#585](#585)) - Ignore input->output links in SiblingSubgraph::try_new_dataflow_subgraph ([#589](#589)) - Enforce covariance of SiblingMut::RootHandle ([#594](#594)) - Erratic stack overflow in infer.rs (live_var) ([#638](#638)) - Work harder in variable instantiation ([#591](#591)) - Actually add the error type to prelude ([#672](#672)) - Serialise dynamically computed opaqueOp signatures ([#690](#690)) - FuncDefns don't require that their extensions match their children ([#688](#688)) - Binary compute_signature returning a PolyFuncType with binders ([#710](#710)) - Use correct number of args for int ops ([#723](#723)) - [**breaking**] Add serde tag to TypeParam enum ([#722](#722)) - Allow widening and narrowing to same width. ([#735](#735)) - Case node should not have an external signature ([#749](#749)) - [**breaking**] Normalize input/output value/static/other ports in `OpType` ([#783](#783)) - No dataflow_signature for block types ([#792](#792)) - Ignore unsupported test in miri ([#794](#794)) - Include schema rather than read file ([#807](#807)) ### Documentation - Add operation constraint to quantum extension ([#543](#543)) - Coverage check section in DEVELOPMENT.md ([#648](#648)) - Remove "quantum extension" from HUGR spec. ([#694](#694)) - Improve crate-level docs, including example code. ([#698](#698)) - Spec cleanups and clarifications ([#742](#742)) - Spec clarifications ([#738](#738)) - Spec updates ([#741](#741)) - [spec] Remove references to causal cone and Order edges from Input ([#762](#762)) - Mention experimental inference in readme ([#800](#800)) - Collection of spec updates for 0.1 ([#801](#801)) - Add schema v0 ([#805](#805)) - Update spec wrt. polymorphism ([#791](#791)) ### Features - Derive things for builder structs ([#229](#229)) - Return a slice of types from the signature ([#238](#238)) - Move `dot_string` to `HugrView` ([#271](#271)) - [**breaking**] Change `TypeParam::USize` to `TypeParam::BoundedNat` and use in int extensions ([#445](#445)) - TKET2 compatibility requirements ([#450](#450)) - Split methods between `HugrMut` and `HugrMutInternals` ([#441](#441)) - Add `HugrView::node_connections` to get all links between nodes ([#460](#460)) - Location information in extension inference error ([#464](#464)) - Add T, Tdg, X gates ([#466](#466)) - [**breaking**] Add `ApplyResult` associated type to `Rewrite` ([#472](#472)) - Implement rewrite `IdentityInsertion` ([#474](#474)) - Instantiate inferred extensions ([#461](#461)) - Default DFG builders to open extension sets ([#473](#473)) - Some helper methods ([#482](#482)) - Extension inference for conditional nodes ([#465](#465)) - Add ConvexChecker ([#487](#487)) - Add clippy lint for mut calls in a debug_assert ([#488](#488)) - Default more builder methods to open extension sets ([#492](#492)) - Port is serializable ([#489](#489)) - More general portgraph references ([#494](#494)) - Add extension deltas to CFG ops ([#503](#503)) - Implement petgraph traits on Hugr ([#498](#498)) - Make extension delta a parameter of CFG builders ([#514](#514)) - [**breaking**] SiblingSubgraph does not borrow Hugr ([#515](#515)) - Validate TypeArgs to ExtensionOp ([#509](#509)) - Derive Debug & Clone for `ExtensionRegistry`. ([#530](#530)) - Const nodes are built with some extension requirements ([#527](#527)) - Some python errors and bindings ([#533](#533)) - Insert_hugr/insert_view return node map ([#535](#535)) - Add `SiblingSubgraph::try_from_nodes_with_checker` ([#547](#547)) - PortIndex trait for undirected port parameters ([#553](#553)) - Insert/extract subgraphs from a HugrView ([#552](#552)) - Add `new_array` operation to prelude ([#544](#544)) - Add a `DataflowParentID` node handle ([#559](#559)) - Make TypeEnum and it's contents public ([#558](#558)) - Optional direction check when querying a port index ([#566](#566)) - Extension inference for CFGs ([#529](#529)) - Better subgraph verification errors ([#587](#587)) - Compute affected nodes for `SimpleReplacement` ([#600](#600)) - Move `SimpleReplace::invalidation_set` to the `Rewrite` trait ([#602](#602)) - [**breaking**] Resolve extension ops (mutating Hugr) in (infer_and_->)update_validate ([#603](#603)) - Add accessors for node index and const values ([#605](#605)) - [**breaking**] Expose the value of ConstUsize ([#621](#621)) - Nicer debug and display for core types ([#628](#628)) - [**breaking**] Static checking of Port direction ([#614](#614)) - Builder and HugrMut add_op_xxx default to open extensions ([#622](#622)) - [**breaking**] Add panicking integer division ops ([#625](#625)) - Add hashable `Angle` type to Quantum extension ([#608](#608)) - [**breaking**] Remove "rotations" extension. ([#645](#645)) - Port.as_directed to match on either direction ([#647](#647)) - Impl GraphRef for PetgraphWrapper ([#651](#651)) - Provide+implement Replace API ([#613](#613)) - Require the node's metadata to always be a Map ([#661](#661)) - [**breaking**] Polymorphic function types (inc OpDefs) using dyn trait ([#630](#630)) - Make prelude error type public ([#669](#669)) - Shorthand for retrieving custom constants from `Const`, `Value` ([#679](#679)) - [**breaking**] HugrView API improvements ([#680](#680)) - Make FuncDecl/FuncDefn polymorphic ([#692](#692)) - [**breaking**] Simplify `SignatureFunc` and add custom arg validation. ([#706](#706)) - [**breaking**] Drop the `pyo3` feature ([#717](#717)) - [**breaking**] `OpEnum` trait for common opdef functionality ([#721](#721)) - MakeRegisteredOp trait for easier registration ([#726](#726)) - Getter for `PolyFuncType::body` ([#727](#727)) - `Into<OpType>` for custom ops ([#731](#731)) - Always require a signature in `OpaqueOp` ([#732](#732)) - Values (and hence Consts) know their extensions ([#733](#733)) - [**breaking**] Use ConvexChecker trait ([#740](#740)) - Custom const for ERROR_TYPE ([#756](#756)) - Implement RemoveConst and RemoveConstIgnore ([#757](#757)) - Constant folding implemented for core and float extension ([#769](#769)) - Constant folding for arithmetic conversion operations ([#720](#720)) - DataflowParent trait for getting inner signatures ([#782](#782)) - Constant folding for logic extension ([#793](#793)) - Constant folding for list operations ([#795](#795)) - Add panic op to prelude ([#802](#802)) - Const::from_bool function ([#803](#803)) ### HugrMut - Validate nodes for set_metadata/get_metadata_mut, too ([#531](#531)) ### HugrView - Validate nodes, and remove Base ([#523](#523)) ### Miscellaneous Tasks - [**breaking**] Update portgraph 0.10 and pyo3 0.20 ([#612](#612)) - [**breaking**] Hike MSRV to 1.75 ([#761](#761)) ### Performance - Use lazy static definittion for prelude registry ([#481](#481)) ### Refactor - Move `rewrite` inside `hugr`, `Rewrite` -> `Replace` implementing new 'Rewrite' trait ([#119](#119)) - Use an excluded upper bound instead of max log width. ([#451](#451)) - Add extension info to `Conditional` and `Case` ([#463](#463)) - `ExtensionSolution` only consists of input extensions ([#480](#480)) - Remove builder from more DFG tests ([#490](#490)) - Separate hierarchy views ([#500](#500)) - [**breaking**] Use named struct for float constants ([#505](#505)) - Allow NodeType::new to take any Into<Option<ExtensionSet>> ([#511](#511)) - Move apply_rewrite from Hugr to HugrMut ([#519](#519)) - Use SiblingSubgraph in SimpleReplacement ([#517](#517)) - CFG takes a FunctionType ([#532](#532)) - Remove check_custom_impl by inlining into check_custom ([#604](#604)) - Insert_subgraph just return HashMap, make InsertionResult new_root compulsory ([#609](#609)) - [**breaking**] Rename predicate to TupleSum/UnitSum ([#557](#557)) - Simplify infer.rs/report_mismatch using early return ([#615](#615)) - Move the core types to their own module ([#627](#627)) - Change &NodeType->&OpType conversion into op() accessor ([#623](#623)) - Infer.rs 'fn results' ([#631](#631)) - Be safe ([#637](#637)) - NodeType constructors, adding new_auto ([#635](#635)) - Constraint::Plus stores an ExtensionSet, which is a BTreeSet ([#636](#636)) - [**breaking**] Remove `SignatureDescription` ([#644](#644)) - [**breaking**] Remove add_op_<posn> by generalizing add_node_<posn> with "impl Into" ([#642](#642)) - Rename accidentally-changed Extension::add_node_xxx back to add_op ([#659](#659)) - [**breaking**] Remove quantum extension ([#670](#670)) - Use type schemes in extension definitions wherever possible ([#678](#678)) - [**breaking**] Flatten `Prim(Type/Value)` in to parent enum ([#685](#685)) - [**breaking**] Rename `new_linear()` to `new_endo()`. ([#697](#697)) - Replace NodeType::signature() with io_extensions() ([#700](#700)) - Validate ExtensionRegistry when built, not as we build it ([#701](#701)) - [**breaking**] One way to add_op to extension ([#704](#704)) - Remove Signature struct ([#714](#714)) - Use `MakeOpDef` for int_ops ([#724](#724)) - [**breaking**] Use enum op traits for floats + conversions ([#755](#755)) - Avoid dynamic dispatch for non-folding operations ([#770](#770)) - Simplify removeconstignore verify ([#768](#768)) - [**breaking**] Unwrap BasicBlock enum ([#781](#781)) - Make clear const folding only for leaf ops ([#785](#785)) - [**breaking**] `s/RemoveConstIgnore/RemoveLoadConstant` ([#789](#789)) - Put extension inference behind a feature gate ([#786](#786)) ### SerSimpleType - Use Vec not TypeRow ([#381](#381)) ### SimpleReplace+OutlineCfg - Use HugrMut methods rather than .hierarchy/.graph ([#280](#280)) ### Testing - Update inference test to not use DFG builder ([#550](#550)) - Strengthen "failing_sccs_test", rename to "sccs" as it's not failing! ([#660](#660)) - [**breaking**] Improve coverage in signature and validate ([#643](#643)) - Use insta snapshots to add dot_string coverage ([#682](#682)) - Miri ignore file-opening test ([#684](#684)) - Unify the serialisation tests ([#730](#730)) - Add schema validation to roundtrips ([#806](#806)) ### `ConstValue - :F64` and `OpaqueOp::new` ([#206](#206)) ### Cleanup - Remove outdated comment ([#536](#536)) ### Cosmetic - Format + remove stray TODO ([#444](#444)) ### Doc - Crate name as README title + add reexport ([#199](#199)) ### S/EdgeKind - :Const/EdgeKind::Static/ ([#201](#201)) ### Simple_replace.rs - Use HugrMut::remove_node, includes clearing op_types ([#242](#242)) ### Spec - Remove "Draft 3" from title of spec document. ([#590](#590)) - Rephrase confusing paragraph about TailLoop inputs/outputs ([#567](#567)) ### Src/ops/validate.rs - Common-up some type row calculations ([#254](#254)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/). --------- Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Agustín Borgna <[email protected]>

ss2165 force-pushed the feat/more-hugrview branch 10 times, most recently from d276e44 to abad4c7 Compare November 13, 2023 15:09

ss2165 marked this pull request as ready for review November 13, 2023 15:11

ss2165 requested a review from aborgna-q November 13, 2023 15:11

ss2165 added 12 commits November 13, 2023 15:51

refactor!: move non_df_count to OpTrait

cd5e544

Closes #521

fix: builder call port calculation incorrect

2b1bd37

refactor!: remove static_input

b959e16

Since it only applies to LoadConst and Call, use special case methods, and a tag to capture these ops.

refactor!: clearer OpType port/kind method names

29350ee

And addds `static_input_port` method. Closes Clean up OpType methods #495

feat: iterator over all ports connected to node

7477cea

Uses return position impl because it will be stable soon and its much nicer. Closes #655

feat: static_source for getting static source node

e6cc8e1

refactor!: rename FunctionType::get -> FunctionType::port_type

11ef7ba

feat: add in/out_port_type to FunctionType

0437efc

feat: HugrView::signature(Node) and port+type iterators

47d4646

feat: single_source/target for when you know only one connected port

2ffd29a

Closes Shorthand HugrView method for "connected port" #499

feat: static_targets and OpType::static_output_port

da9aea8

feat: dataflow_ports_only function to filter node-ports

b2a2ae0

ss2165 force-pushed the feat/more-hugrview branch from abad4c7 to b2a2ae0 Compare November 13, 2023 15:51

ss2165 added 2 commits November 14, 2023 12:14

refactor!: OpType::signature returns option (non-dataflow ops don't r…

b4db711

…eturn signature)

refactor!: OpType::signature returns option (non-dataflow ops don't r…

39e1490

…eturn signature)

acl-cqc reviewed Nov 14, 2023

View reviewed changes

refactor: use enum to simplify optype pattern matches

5fd1035

ss2165 commented Nov 14, 2023

View reviewed changes

ss2165 added 13 commits November 14, 2023 15:05

chore: remove paste from dev-dependencies

09844a1

feat: all_linked_ports

b124160

address minor review comments

328ce45

feat!: single_linked_port, single_linked_input, single_linked_output

44cb5b0

refactor!: allow chaining with dataflow_ports_only

f7bfb46

feat!: parametric all_linked_ports

920880e

refactor: simpler static_input_port

2d1e47c

refactor: remove uncessary trait impls for ExternalOp

7ea609a

simplify static i/o tags

37ceea8

refactor: nicer code in signature.rs

d31794d

refactor!: swap signature and dataflow_signature

4636bf7

refactor!: non_df_port_count just returns usize

0a6bb8a

Merge branch 'main' into feat/more-hugrview

9556156

ss2165 requested a review from aborgna-q November 14, 2023 17:33

ss2165 added 4 commits November 14, 2023 17:36

fix: put correct compile flags in

f091b5a

fix doclinks

451ce23

Merge branch 'main' into feat/more-hugrview

c1822db

Merge branch 'main' into feat/more-hugrview

65ed37a

aborgna-q approved these changes Nov 15, 2023

View reviewed changes

ss2165 added this pull request to the merge queue Nov 15, 2023

Merged via the queue into main with commit e943fdc Nov 15, 2023
10 checks passed

ss2165 deleted the feat/more-hugrview branch November 15, 2023 18:00

acl-cqc mentioned this pull request Nov 15, 2023

refactor: Replace NodeType::signature() with io_extensions() #700

Merged

aborgna-q mentioned this pull request Nov 17, 2023

chore: update deps CQCL/tket2#247

Merged

github-actions bot mentioned this pull request Jan 3, 2024

chore: release v0.1.0 #766

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: HugrView API improvements #680

feat!: HugrView API improvements #680

ss2165 commented Nov 11, 2023 •

edited

Loading

codecov bot commented Nov 11, 2023 •

edited

Loading

acl-cqc Nov 14, 2023

ss2165 Nov 14, 2023

acl-cqc Nov 14, 2023

ss2165 Nov 14, 2023

ss2165 Nov 14, 2023

acl-cqc Nov 14, 2023

aborgna-q left a comment •

edited

Loading

feat!: HugrView API improvements #680

feat!: HugrView API improvements #680

Conversation

ss2165 commented Nov 11, 2023 • edited Loading

codecov bot commented Nov 11, 2023 • edited Loading

Codecov Report

acl-cqc Nov 14, 2023

Choose a reason for hiding this comment

ss2165 Nov 14, 2023

Choose a reason for hiding this comment

acl-cqc Nov 14, 2023

Choose a reason for hiding this comment

ss2165 Nov 14, 2023

Choose a reason for hiding this comment

ss2165 Nov 14, 2023

Choose a reason for hiding this comment

acl-cqc Nov 14, 2023

Choose a reason for hiding this comment

aborgna-q left a comment • edited Loading

Choose a reason for hiding this comment

ss2165 commented Nov 11, 2023 •

edited

Loading

codecov bot commented Nov 11, 2023 •

edited

Loading

aborgna-q left a comment •

edited

Loading