Docs for core data structures #232
Conversation
|
@0x0f0f0f I think this is ready to merge, as long as I have not said anything erroneous in the docstrings. If I have more docstrings, I'll open a new PR; I think it's best to not have an uber-PR. |
src/EGraphs/egraph.jl
Outdated
| """ | ||
| hashcons the constants in the e-graph | ||
|
|
||
| For performance reasons, the "head" of an e-node is hashed and the |
There was a problem hiding this comment.
| For performance reasons, the "head" of an e-node is hashed and the | |
| For performance reasons, the "head" of an e-node is hashed to be stored in a flat vector of unsigned integers ([`VecExpr`](@ref)) and the |
src/EGraphs/egraph.jl
Outdated
| hashcons the constants in the e-graph | ||
|
|
||
| For performance reasons, the "head" of an e-node is hashed and the | ||
| value is stored in this Dict. |
There was a problem hiding this comment.
| value is stored in this Dict. | |
| actual Julia object of the e-node operation is stored in this Dict. |
src/vecexpr.jl
Outdated
| The meaning of the bitflags `isexpr` and `iscall` can be best understood through looking at | ||
| the source for `to_expr(g::EGraph, n::VecExpr)` in `src/EGraphs/egraph.jl`. Namely, | ||
| e-nodes for which `isexpr` is false have no arguments; their only "data" is their head. | ||
| E-nodes for which `isexpr` is true and `iscall` is also true correspond to | ||
| `Expr(:call, head, args...)` expressions, and e-nodes for which `isexpr` is true but | ||
| `iscall` is false correspond to `Expr(head, args...)` expressions. There should | ||
| not be `VecExpr`s with `isexpr = false` but `iscall = true`. |
There was a problem hiding this comment.
Please check the TermInterface source. This corresponds exactly to isexpr and iscall from TermInterface.
There was a problem hiding this comment.
Ah, OK! I'll update these docs to point to TermInterface.
src/vecexpr.jl
Outdated
| The "signature" of an expression seems to in practice be computed as the hash of the head combined | ||
| with the number of arguments (the arity). See: [`addexpr!`]() in `src/EGraphs/egraph.jl`. |
There was a problem hiding this comment.
| The "signature" of an expression seems to in practice be computed as the hash of the head combined | |
| with the number of arguments (the arity). See: [`addexpr!`]() in `src/EGraphs/egraph.jl`. | |
| The "signature" of an expression is computed as the hash of the head combined | |
| with the number of arguments (the arity). See: [`addexpr!`]() in `src/EGraphs/egraph.jl`. |
src/vecexpr.jl
Outdated
| Perhaps in the future, signatures could also involve type information, e.g. to disambiguate | ||
| overloaded heads? Signatures are used in the `classes_by_op` dictionary in a e-graph, | ||
| so that when you are matching for `(a + b)` you can iterate over all of the e-classes | ||
| that have some e-node with `(+, 2)` as its signature. |
There was a problem hiding this comment.
Yeah, this was more me documenting my own thoughts... I've updated this.
src/vecexpr.jl
Outdated
| so that when you are matching for `(a + b)` you can iterate over all of the e-classes | ||
| that have some e-node with `(+, 2)` as its signature. | ||
|
|
||
| It also seems like the signature of a constant is `0`. |
| Note: this is really more of a stack than a queue, because it is LIFO (last in | ||
| first out) rather than FIFO (first in, last out). That is, | ||
|
|
||
| ```julia | ||
| q = UniqueQueue{Int}() | ||
| push!(q, 1) | ||
| push!(q, 2) | ||
| assert(pop!(q) == 2) | ||
| assert(pop!(q) == 1) | ||
| ``` |
There was a problem hiding this comment.
Nice catch. This is modeled after egg's unique queue. We should check egg's source code and see if it's FIFO or LIFO. Could mean a big performance gain if they're going FIFO and we're doing LIFO, we should try to align with their implementation.
There was a problem hiding this comment.
It does seem like they are going FIFO:
According to https://doc.rust-lang.org/std/collections/struct.VecDeque.html, push_back and pop_front is FIFO, and also VecDeque is implemented with a ring buffer.
src/EGraphs/unionfind.jl
Outdated
| Potential optimization: | ||
|
|
||
| ```julia | ||
| j = i | ||
| while j != uf.parents[j] | ||
| j = uf.parents[j] | ||
| end | ||
| root = j | ||
| while i != uf.parents[i] | ||
| uf.parents[i] = root | ||
| i = uf.parents[i] | ||
| end | ||
| root | ||
| ``` |
There was a problem hiding this comment.
This implementation detail should go in a comment and not in the docstring
src/EGraphs/unionfind.jl
Outdated
| """ | ||
| UnionFind() | ||
|
|
||
| Creates a new empty unionfind. | ||
| """ |
There was a problem hiding this comment.
| """ | |
| UnionFind() | |
| Creates a new empty unionfind. | |
| """ | |
| """ | |
| UnionFind() | |
| Creates a new empty unionfind. | |
| """ |
Can you move it to the struct docstring please?
src/EGraphs/egraph.jl
Outdated
| """ | ||
| Look up a grounded pattern. | ||
| """ |
There was a problem hiding this comment.
| """ | |
| Look up a grounded pattern. | |
| """ | |
| """ | |
| Look up a ground pattern. | |
| """ |
Co-authored-by: Alessandro Cheli <[email protected]>
Work in progress.