-
Notifications
You must be signed in to change notification settings - Fork 7
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
Update definition of Replace (and clarify SimpleReplace) #596
Conversation
…y revert" This reverts commit f3e0150a5d5ec889bff68e93844255fb1a8f7c68.
Given a `Const<T>` node `c` and a DSG `P`, add `c` as a child of `P`, | ||
inserting an Order edge from the Input under `P` to `c`. | ||
Given a `Const<T>` node `c` and a container node `P` (either a `Module`, | ||
a `CFG` node or a dataflow container), add `c` as a child of `P`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
specification/hugr.md
Outdated
- a map $\nu\_\textrm{inp}: \textrm{inp}\_H(T \setminus \\{\texttt{Input}\\}) \to \textrm{inp}\_{\Gamma}(S)$; | ||
- a map $\nu_\textrm{out}: \textrm{out}_{\Gamma}(S) \to \textrm{out}_H(T \setminus \\{\texttt{Output}\\})$. | ||
- a map $\nu\_\textrm{inp}: \textrm{inp}\_H(T \setminus \\{\texttt{Input}\\}) \to \textrm{inp}\_{\Gamma}(S)$ (*note* in order to produce a valid Hugr, all keys must be present; and all possible values must be present exactly once unless Copyable); | ||
- a map $\nu_\textrm{out}: \textrm{out}_{\Gamma}(S) \to \textrm{out}_H(T \setminus \\{\texttt{Output}\\})$ (*note* (similarly) in order to produce a valid hugr, all keys $\textrm{out}_{\Gamma}(S)$ must be present; and each possible value must be either Copyable and/or present exactly once. Any possible value not present could just be omitted from $H$...). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are quite inconsistent with capitalization of hugr/Hugr/HUGR, but this can be fixed later.
This seems like a useful set of clarifications, though more changes may be needed... |
Ok, I've had a pass at updating Replace.
|
specification/hugr.md
Outdated
- a list $\mu\_\textrm{out}$ of `NewEdgeSpec` which all have their `SrcNode`in | ||
$G$ and `TgtNode` in $\Gamma \setminus S^*$ (and `TgtNode` has an existing | ||
$G$ and `TgtNode` in $\Gamma \setminus R$ (and `TgtNode` has an existing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to be more precise about the target port/whatever having an existing incoming edge, but that's a little awkward. I'm still wondering about a bigger restructure of edges from being two "lists", but I think the "well-formedness" requirements are so complex anyway that one might as well give up....
One thing I think one might want is to add new edges between nodes in `$\Gamma \setminus R$ i.e. where the source has been removed, the "new" source might also be in the remainder rather than new in the replacement. Forcing all such edges to be "passed through" Input -> Output doesn't work if
One possibility might be to allow mu_out to have any SrcNode, although that's tricky because until we've copied
A third set of new edges?? There may be call for being able to add arbitrary new Order edges, too....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course one can always use HugrMut::connect
, I guess we can add this later if we really find a need
- a list $\mu\_\textrm{inp}$ of `NewEdgeSpec` which all have their `TgtNode`in | ||
$G$ and `SrcNode` in $\Gamma \setminus S^*$; | ||
$G$ and `SrcNode` in $\Gamma \setminus R$; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this change is allowing SrcNode
to be in S* ∩ X*. OK I think I see why this is allowed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you move a subtree from the old Hugr to a new location....
- There won't be any Ext edges out of it (because they'd only go to siblings, i.e. new bits added INTO the subtree you've moved, and Replace doesn't let you add any such)
- But you might need to add Ext edges into it (if the bit you kept, had an Ext edge from a bit you removed)
- And, you could have Dom edges out of the bit you're moving into new nodes, or Dom edges in as previous point.
Should I expand, do you think? I admit that change looks quite small but has bigger ramifications!
@@ -1253,8 +1261,10 @@ The new hugr is then derived as follows: | |||
copy of the `SrcNode` of $\sigma\_\mathrm{out}$ according to the specification $\sigma\_\mathrm{out}$. | |||
The target port must have an existing edge whose source is in $R$; this edge | |||
is removed. | |||
4. For each $(n, t = T(n))$, append the copy of $n$ to the list | |||
of children of $t$ (adding a hierachy edge from $t$ to $n$). | |||
4. Let $N$ be the ordered list of the copies made in $\Gamma$ of the children of the root node of $G$. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit more complex than append-to-end, but I think it may be necessary for replacing particular children of Conditionals (even without changing their number); CFG entry nodes; and Input/Output nodes. Generally you'll need to provide exactly one Input if-and-only-if you remove an Input, sort of thing, but it seems reasonable that you might???
T
map)