-
Notifications
You must be signed in to change notification settings - Fork 7
Redesign Proposal #22
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
Changes from all commits
5f0b5d9
2052ebd
3cb7568
f235686
bc1ec14
bcc4574
e2aef1a
47610e7
85d0931
62ff72a
59da1f7
eef8273
a3eacea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,27 @@ | ||
| """ | ||
| This module defines a contains definitions for common functions that are useful | ||
| for symbolic expression manipulation. Its purpose is to provide a shared | ||
| interface between various symbolic programming Julia packages. | ||
|
|
||
| This is currently borrowed from TermInterface.jl. If you want to use | ||
| Metatheory.jl, please use this internal interface, as we are waiting that a | ||
| redesign proposal of the interface package will reach consensus. When this | ||
| happens, this module will be moved back into a separate package. | ||
|
|
||
| See https://github.com/JuliaSymbolics/TermInterface.jl/pull/22 | ||
| """ | ||
| module TermInterface | ||
|
|
||
| """ | ||
| istree(x) | ||
|
|
||
| Returns `true` if `x` is a term. If true, `operation`, `arguments` | ||
| must also be defined for `x` appropriately. | ||
| Returns `true` if `x` is a term. If true, `operation`, `arguments` and | ||
| `is_function_call` must also be defined for `x` appropriately. | ||
| """ | ||
| istree(x) = false | ||
| export istree | ||
|
|
||
|
|
||
| """ | ||
| symtype(x) | ||
|
|
||
|
|
@@ -22,6 +35,7 @@ function symtype(x) | |
| end | ||
| export symtype | ||
|
|
||
|
|
||
| """ | ||
| issym(x) | ||
|
|
||
|
|
@@ -31,44 +45,30 @@ on `x` and must return a Symbol. | |
| issym(x) = false | ||
| export issym | ||
|
|
||
| """ | ||
| exprhead(x) | ||
|
|
||
| If `x` is a term as defined by `istree(x)`, `exprhead(x)` must return a symbol, | ||
| corresponding to the head of the `Expr` most similar to the term `x`. | ||
| If `x` represents a function call, for example, the `exprhead` is `:call`. | ||
| If `x` represents an indexing operation, such as `arr[i]`, then `exprhead` is `:ref`. | ||
| Note that `exprhead` is different from `operation` and both functions should | ||
| be defined correctly in order to let other packages provide code generation | ||
| and pattern matching features. | ||
| """ | ||
| function exprhead end | ||
| export exprhead | ||
|
|
||
|
|
||
| """ | ||
| operation(x) | ||
|
|
||
| If `x` is a term as defined by `istree(x)`, `operation(x)` returns the | ||
| head of the term if `x` represents a function call, for example, the head | ||
| is the function being called. | ||
| If `x` is a term as defined by `istree(x)`, `operation(x)` returns the operation of the | ||
| term. If `x` represents a function call term like `f(a,b)`, the operation | ||
| is the function being called, `f`. | ||
| """ | ||
| function operation end | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again should not be in this package.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I believe this one should stay in this package.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This package is used to define an AST interface so that pattern matchers can match against the AST and traverse it. I think it's a little bit of a mistake to introduce TL;DR: I don't think we can define what the new meaning of
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I guess most of what the (old/current) dependents of this package do:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can use trait like abstractrees? Will make a larger comment below
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @shashi, do we agree now that |
||
| export operation | ||
|
|
||
|
|
||
| """ | ||
| arguments(x) | ||
|
|
||
| Get the arguments of `x`, must be defined if `istree(x)` is `true`. | ||
| Get the arguments of a term `x`, must be defined if `istree(x)` is `true`. | ||
| """ | ||
| function arguments end | ||
| export arguments | ||
|
|
||
|
|
||
| """ | ||
| unsorted_arguments(x::T) | ||
|
|
||
| If x is a term satisfying `istree(x)` and your term type `T` orovides | ||
| If x is a term satisfying `istree(x)` and your term type `T` provides | ||
| and optimized implementation for storing the arguments, this function can | ||
| be used to retrieve the arguments when the order of arguments does not matter | ||
| but the speed of the operation does. | ||
|
|
@@ -83,7 +83,7 @@ export unsorted_arguments | |
| Returns the number of arguments of `x`. Implicitly defined | ||
| if `arguments(x)` is defined. | ||
| """ | ||
| arity(x) = length(arguments(x)) | ||
| arity(x)::Int = length(arguments(x)) | ||
| export arity | ||
|
|
||
|
|
||
|
|
@@ -102,29 +102,29 @@ export metadata | |
| Returns a new term which has the structure of `x` but also has | ||
| the metadata `md` attached to it. | ||
| """ | ||
| function metadata(x, data) | ||
| error("Setting metadata on $x is not possible") | ||
| end | ||
| function metadata(x, data) end | ||
|
|
||
|
|
||
| """ | ||
| similarterm(x, head, args, symtype=nothing; metadata=nothing, exprhead=:call) | ||
| maketerm(T::Type, operation, arguments; type=Any, metadata=nothing) | ||
|
|
||
| Returns a term that is in the same closure of types as `typeof(x)`, | ||
| with `head` as the head and `args` as the arguments, `type` as the symtype | ||
| and `metadata` as the metadata. By default this will execute `head(args...)`. | ||
| `x` parameter can also be a `Type`. The `exprhead` keyword argument is useful | ||
| when manipulating `Expr`s. | ||
| Has to be implemented by the provider of the expression type T. | ||
| Returns a term that is in the same closure of types as `T`, | ||
| with `operation` as the operation and `arguments` as the arguments, `type` as the symtype | ||
| and `metadata` as the metadata. | ||
| """ | ||
| function similarterm(x, head, args, symtype = nothing; metadata = nothing, exprhead = nothing) | ||
| head(args...) | ||
| end | ||
| function maketerm end | ||
| export maketerm | ||
|
|
||
|
|
||
| export similarterm | ||
|
|
||
| include("utils.jl") | ||
| """ | ||
| node_count(t) | ||
| Count the nodes in a symbolic expression tree satisfying `istree` and `arguments`. | ||
| """ | ||
| node_count(t) = istree(t) ? reduce(+, node_count(x) for x in arguments(t), init in 0) + 1 : 1 | ||
| export node_count | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fine, though I wonder if AbstractTrees could help here. |
||
| include("expr.jl") | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This approach to building ASTs results in huge slowdowns in compilers, because compilers specialize on each combination of node types. I appreciate the idea of adding an optional AST easy implementation feature, could we add it in a separate PR? A few ideas:
If we feel like an optimized AST implementation is out of scope, we shouldn't include programming patterns that will inconvenience users later on with performance issues. |
||
| end # module | ||
|
|
||
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,3 @@ | ||
| using TermInterface | ||
| using Test | ||
| using TermInterface, Test | ||
|
|
||
| @testset "Expr" begin | ||
| ex = :(f(a, b)) | ||
| @test operation(ex) == :f | ||
| @test arguments(ex) == [:a, :b] | ||
| @test exprhead(ex) == :call | ||
| @test ex == similarterm(ex, :f, [:a, :b]) | ||
|
|
||
| ex = :(arr[i, j]) | ||
| @test operation(ex) == getindex | ||
| @test arguments(ex) == [:arr, :i, :j] | ||
| @test exprhead(ex) == :ref | ||
| @test ex == similarterm(ex, :ref, [:arr, :i, :j]; exprhead = :ref) | ||
| @test ex == similarterm(ex, :ref, [:arr, :i, :j]) | ||
| end | ||
| @test true |
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 exactly what this redesign had hoped to avoid! We don't want
headto be the function being called. We want it to be:calland we wantchildren(x)[1]to be the function being called.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 thought we wanted to avoid the concept of
:callas "exprhead".In my latest commit
headis the oldoperation, andchildrenis the oldarguments.If we add both
head,childrenandoperation,argumentsMetatheory.jl would just rely onoperationandargumentsfor pattern matching. I guess the same for SU.But what about SymbolicUtils terms?
t = f(a,b)in SU would haveoperation(t) == f,arguments(t) == [a,b],children(t) = [f,a,b], what abouthead(t)? Should it beSUHead()?I kinda dislike the idea that the users should define a struct to define the
headof an AST node, all the information required to inspect, manipulate and create new terms is already contained in the type of the term.