-
Notifications
You must be signed in to change notification settings - Fork 17
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
Graph-like expressions #56
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pull Request Test Coverage Report for Build 7247053604
💛 - Coveralls |
Benchmark Results
|
MilesCranmer
force-pushed
the
MilesCranmer/issue14
branch
from
November 28, 2023 09:34
b78fa41
to
b1431e6
Compare
MilesCranmer
force-pushed
the
MilesCranmer/issue14
branch
from
December 17, 2023 22:00
e4db696
to
83f0a61
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This reverts commit 83f0a61.
MilesCranmer
force-pushed
the
MilesCranmer/issue14
branch
from
December 19, 2023 04:57
c24920e
to
627091f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This WIP PR will implement remaining methods for trees that allow two or more parents to share a node. This means we can re-use subexpressions in a single expression. @AlCap23 This should partially address some of #14.
Comments:
GraphNode
type which inherits from a newAbstractExpressionNode
. It is assumed that expressions made withGraphNode
are to be treated as having shared subexpressions (so requires storing a stack of node IDs when counting, etc.), but those made withNode
are treated like trees (even if there are some shared subexpressions).Node
will destroy any shared subexpressions.tree_mapreduce
in the form of a passed functionf_on_shared
which takes two arguments: the result of the function evaluation, and whether it has already been passed. So, for instance,count_nodes
can definef_on_shared=(result, is_shared) -> is_shared ? 0 : result
, meaning it will avoid double-counting nodes.IdDict
toDict{UInt}
because surprisingly the latter is faster for doing the exact same task! I looked into why this is and apparentlyIdDict
does not specialize its methods on the input value... So it's actually faster to just pass theobjectid
manually to a regularDict
. Very weird.Here's a to-do list of functions remaining:
copy_node
convert
simplify_tree!
combine_operators
(Conceptually not possible - I throw an error instead)string_tree
{ ... }
. It is easy to pattern match with your eyes and figure out where it is shared from.z1 = ...; y=sin(z1)*z1
or something.print_tree
show
count_nodes
count_depth
(will work as-is)count_constants
index_constants
get_constants
set_constants!
set_node!
(will work as-is)filter
collect
map
filter_map
count
foreach
collect
sum
mapreduce
iterate
(Not possible)length
copy
hash
==
(requires exact structure when comparingGraphNode
. If want to compare without structure, just doNode(t1) == Node(t2)
.compute_complexity
Then these ones:
To-do for detailed testing:
copy_node
convert
simplify_tree!
print
andshow
count_nodes
count_depth
count_constants
index_constants
get_constants
set_constants!
set_node!
hash
==
filter
collect
map
filter_map
count
foreach
collect
sum
mapreduce
iterate
length
copy
We also need to: