docs: Spec updates #741

cqc-alec · 2023-12-14T16:36:48Z

Following Will's review.

ss2165 · 2023-12-14T16:39:09Z

couldn't a passthrough<T> just be a normal basic block that wires inputs to outputs?

codecov · 2023-12-14T16:40:24Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0edee65) 84.03% compared to head (90a98fd) 84.10%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #741      +/-   ##
==========================================
+ Coverage   84.03%   84.10%   +0.06%     
==========================================
  Files          67       67              
  Lines       13189    13189              
  Branches    13189    13189              
==========================================
+ Hits        11083    11092       +9     
+ Misses       1320     1311       -9     
  Partials      786      786

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

cqc-alec · 2023-12-14T16:41:27Z

couldn't a passthrough<T> just be a normal basic block that wires inputs to outputs?

Yes that's the idea, but I needed a name for it (it isn't a new node type).

ss2165 · 2023-12-14T17:03:02Z

Ah ok, but just to double check identity<T> is a node type right?

cqc-alec · 2023-12-14T17:08:21Z

Ah ok, but just to double check identity<T> is a node type right?

Hmm, yes. This is confusing I agree. Will try to make clearer.

acl-cqc

Thanks @alec - in general, like it :), some thoughts about passthrough and "adoptions"

acl-cqc · 2024-01-02T10:02:36Z

specification/hugr.md

+by the `Replace` operation). To fully specify the operation, some further data
+are needed:
+
+  - The replacement may containg container nodes with no children, which replace


containg -> contain or perhaps include. However I'm less sure about these "replace"ing removed container nodes. They do WRT the replaced node's children, but not necessarily in any other way (e.g. they may have different parents, siblings, incoming/outgoing dataflow/controlflow edges to the "replaced" container nodes).

How about:

- The replacement may include container nodes with no children, which adopt the children of removed container nodes and prevent those children being removed

? - note we haven't said (yet / in the summary) that children of removed nodes are themselves removed

acl-cqc · 2024-01-02T10:03:06Z

specification/hugr.md

@@ -408,6 +408,9 @@ Some normalizations are possible:
  - If the entry node has only one successor and that successor is the
    exit node, the CFG node itself can be removed.

+A `DFB` that simply wires its inputs to its outputs, having no effect on the


Is this a new node type? I think it's just a special case of an existing one, so might be worth clarifying that.

If it is indeed a special case, then it'll still have Input and Output children with dataflow edges between those

But if it's a special kind of node then maybe it has no children. This would be more like Identity, and that'd be nice for uniformity wrt (Remove/Insert)Identity. Note this would be a lot like an Exit node, so maybe we can even remove the Exit node type by requiring that the second child of a CFG node be a passthrough node (even if not necessarily the only one)...???

So the intention was not to define a new node type here, but maybe there is an argument for doing so, especially if it allows us to remove the Exit type. Hmm, that seems like a big change though.

Conversely, maybe identity<T> is not required. It could just be a DFG node with, much like a "passthrough" DFB, all out-ports of the Input node wired to all in-ports of the Output node....

Perhaps the thing to do is to break the passthrough/identity/etc. change out (if it's gonna be big one way or another) and then everything else here seems uncontroversial...

acl-cqc · 2024-01-02T10:12:17Z

specification/hugr.md

@@ -1371,7 +1386,7 @@ nodes.

 The most basic case – replacing a convex set of Op nodes in a DSG with
 another graph of Op nodes having the same signature – is implemented by
-having T map everything to the parent node, and bot(G) is empty.
+`SimpleReplace`.


Hehe! Yes :)

acl-cqc · 2024-01-02T10:13:01Z

specification/hugr.md

@@ -1191,7 +1194,18 @@ The new hugr is then derived as follows:

 ###### `Replace`

-This is the general subgraph-replacement method.
+This is the general subgraph-replacement method. Intuitively, it takes a set of


Great idea to provide an intuition here :)

This reverts commit 44d2ea3.

This reverts commit 37a5f72.

acl-cqc

All good by me, thanks @cqc-alec. Just remove the node about passthrough in the description before you merge :)

Following Will's review.

## 🤖 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 <121866228+aborgna-q@users.noreply.github.com>

cqc-alec added 6 commits December 14, 2023 15:07

Add a bit of intuition about Replace.

370e6dc

Define an identity node for CFGs.

37a5f72

State that acyclicity is preserved by InsertOrder.

d0d0940

Use correct terminology.

1c9853e

Correct typo.

2733432

Fix outdated text.

207079b

cqc-alec requested a review from acl-cqc December 14, 2023 16:36

Merge branch 'main' into spec-updates

8f57eae

cqc-alec added 2 commits December 14, 2023 17:15

Make description of "passthrough" DFBs clearer.

44d2ea3

Merge branch 'main' into spec-updates

6b20e6a

acl-cqc reviewed Jan 2, 2024

View reviewed changes

cqc-alec added 4 commits January 2, 2024 11:39

Merge branch 'main' into spec-updates

00d6aee

Improve explanation of adoption.

cf1d9ac

Revert "Make description of "passthrough" DFBs clearer."

4f61b7f

This reverts commit 44d2ea3.

Revert "Define an identity node for CFGs."

90a98fd

This reverts commit 37a5f72.

cqc-alec requested a review from acl-cqc January 2, 2024 12:59

cqc-alec mentioned this pull request Jan 2, 2024

Define analog for InsertIdentity and RemoveIdentity for CFGs #763

Open

acl-cqc approved these changes Jan 2, 2024

View reviewed changes

cqc-alec added this pull request to the merge queue Jan 3, 2024

Merged via the queue into main with commit 198f221 Jan 3, 2024
11 checks passed

cqc-alec deleted the spec-updates branch January 3, 2024 09:13

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

chore: release v0.1.0 #766

Merged

ss2165 pushed a commit that referenced this pull request Jan 3, 2024

docs: Spec updates (#741)

3193cdb

Following Will's review.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: Spec updates #741

docs: Spec updates #741

cqc-alec commented Dec 14, 2023 •

edited

Loading

ss2165 commented Dec 14, 2023

codecov bot commented Dec 14, 2023 •

edited

Loading

cqc-alec commented Dec 14, 2023

ss2165 commented Dec 14, 2023

cqc-alec commented Dec 14, 2023

acl-cqc left a comment

acl-cqc Jan 2, 2024

acl-cqc Jan 2, 2024

acl-cqc Jan 2, 2024

cqc-alec Jan 2, 2024

acl-cqc Jan 2, 2024

cqc-alec Jan 2, 2024

acl-cqc Jan 2, 2024

acl-cqc Jan 2, 2024

acl-cqc left a comment

docs: Spec updates #741

docs: Spec updates #741

Conversation

cqc-alec commented Dec 14, 2023 • edited Loading

ss2165 commented Dec 14, 2023

codecov bot commented Dec 14, 2023 • edited Loading

Codecov Report

cqc-alec commented Dec 14, 2023

ss2165 commented Dec 14, 2023

cqc-alec commented Dec 14, 2023

acl-cqc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acl-cqc left a comment

Choose a reason for hiding this comment

cqc-alec commented Dec 14, 2023 •

edited

Loading

codecov bot commented Dec 14, 2023 •

edited

Loading