Skip to content

Permit n-argument operators #127

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

Open
wants to merge 53 commits into
base: master
Choose a base branch
from
Open

Permit n-argument operators #127

wants to merge 53 commits into from

Conversation

MilesCranmer
Copy link
Member

@MilesCranmer MilesCranmer commented May 10, 2025

This opens up DynamicExpressions.jl to D-degree nodes via an additional type parameters. This is still a work-in-progress, but I wanted to track the TODO's here.

TODO:

  • Tests
    • Test string paths
    • Random expression evals for arbitrary arity, compare against Julia evaluation
    • 100% diff coverage
  • Performance
    • Evaluation performance (seems to be roughly the same now)
    • Allocation performance (at the moment, it seems to result in double the # of allocations, and 33% greater memory usage)
    • Either split types, or get generic type to match existing performance for
    • Consider using ::Union{Nothing,N} rather than Ref
    • Profile SymbolicRegression.jl against the diff
  • Feature coverage
    • Generic operator enum compatibility
    • Derivative compatibility
    • Preallocation compatibility
    • LoopVectorization.jl compat
    • Bumper.jl compat
    • Verify graph node compat
    • Verify simplification compat
  • Cleaning
    • Simplify evaluation code with generic Base.Cartesian version
    • Manually verify no internal uses of .l and .r (temporarily set these to be errors)
    • Introduce alias Node{T} = NNode{T,2} for full backwards compatibility

@MilesCranmer MilesCranmer changed the base branch from n-arity to master May 10, 2025 13:11
Comment on lines +19 to +20
function OperatorEnum(binary_operators::Tuple, unary_operators::Tuple)
return OperatorEnum((unary_operators, binary_operators))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add a depwarn here

@coveralls
Copy link

coveralls commented May 10, 2025

Pull Request Test Coverage Report for Build 14946536677

Details

  • 246 of 278 (88.49%) changed or added relevant lines in 14 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.9%) to 94.724%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/ExpressionAlgebra.jl 41 42 97.62%
src/OperatorEnum.jl 16 17 94.12%
src/Node.jl 70 72 97.22%
src/ReadOnlyNode.jl 10 16 62.5%
src/Evaluate.jl 16 38 42.11%
Totals Coverage Status
Change from base Build 14729298079: -0.9%
Covered Lines: 2657
Relevant Lines: 2805

💛 - Coveralls

Copy link
Contributor

github-actions bot commented May 10, 2025

Benchmark Results

master b66b178... master / b66b178...
eval/ComplexF32/evaluation 7.31 ± 0.52 ms 7.31 ± 0.59 ms 0.999 ± 0.11
eval/ComplexF64/evaluation 10.9 ± 0.98 ms 10.8 ± 0.78 ms 1.01 ± 0.12
eval/Float32/derivative 11.4 ± 0.54 ms 11.8 ± 0.71 ms 0.973 ± 0.074
eval/Float32/derivative_turbo 11.7 ± 0.84 ms 11.7 ± 0.73 ms 0.995 ± 0.094
eval/Float32/evaluation 2.71 ± 0.27 ms 2.71 ± 0.27 ms 1 ± 0.14
eval/Float32/evaluation_bumper 0.599 ± 0.015 ms 0.607 ± 0.017 ms 0.987 ± 0.037
eval/Float32/evaluation_turbo 0.527 ± 0.033 ms 0.533 ± 0.033 ms 0.99 ± 0.087
eval/Float32/evaluation_turbo_bumper 0.6 ± 0.015 ms 0.606 ± 0.016 ms 0.99 ± 0.036
eval/Float64/derivative 15.3 ± 1.5 ms 15 ± 1.4 ms 1.02 ± 0.14
eval/Float64/derivative_turbo 15.5 ± 1.7 ms 15.1 ± 1.4 ms 1.03 ± 0.15
eval/Float64/evaluation 3.14 ± 0.34 ms 3.14 ± 0.33 ms 1 ± 0.15
eval/Float64/evaluation_bumper 1.25 ± 0.043 ms 1.27 ± 0.044 ms 0.985 ± 0.048
eval/Float64/evaluation_turbo 1.01 ± 0.071 ms 1.03 ± 0.069 ms 0.987 ± 0.096
eval/Float64/evaluation_turbo_bumper 1.25 ± 0.045 ms 1.27 ± 0.045 ms 0.985 ± 0.05
utils/combine_operators/break_sharing 0.039 ± 0.00064 ms 0.0388 ± 0.00073 ms 1.01 ± 0.025
utils/convert/break_sharing 27.1 ± 3.2 μs 25.2 ± 2.6 μs 1.07 ± 0.17
utils/convert/preserve_sharing 0.1 ± 0.0063 ms 0.544 ± 0.011 ms 0.185 ± 0.012
utils/copy/break_sharing 27.8 ± 3.2 μs 25.8 ± 2.6 μs 1.08 ± 0.17
utils/copy/preserve_sharing 0.101 ± 0.006 ms 0.0984 ± 0.0052 ms 1.02 ± 0.082
utils/count_constant_nodes/break_sharing 8.57 ± 0.2 μs 7.99 ± 0.33 μs 1.07 ± 0.05
utils/count_constant_nodes/preserve_sharing 0.0851 ± 0.0044 ms 0.086 ± 0.0042 ms 0.989 ± 0.071
utils/count_depth/break_sharing 10.2 ± 0.35 μs 9.7 ± 0.24 μs 1.06 ± 0.044
utils/count_nodes/break_sharing 8.71 ± 0.2 μs 8.88 ± 0.27 μs 0.981 ± 0.038
utils/count_nodes/preserve_sharing 0.0875 ± 0.0047 ms 0.0853 ± 0.004 ms 1.03 ± 0.073
utils/get_set_constants!/break_sharing 0.0344 ± 0.0019 ms 0.032 ± 0.0019 ms 1.08 ± 0.087
utils/get_set_constants!/preserve_sharing 0.178 ± 0.0085 ms 0.177 ± 0.0085 ms 1.01 ± 0.068
utils/get_set_constants_parametric 0.044 ± 0.0028 ms 0.0442 ± 0.0029 ms 0.996 ± 0.091
utils/has_constants/break_sharing 4.97 ± 0.15 μs 4.62 ± 0.19 μs 1.08 ± 0.055
utils/has_operators/break_sharing 2.3 ± 0.082 μs 2.33 ± 0.094 μs 0.989 ± 0.053
utils/hash/break_sharing 23.3 ± 0.7 μs 21.7 ± 0.79 μs 1.07 ± 0.051
utils/hash/preserve_sharing 0.0993 ± 0.0078 ms 0.0987 ± 0.0054 ms 1.01 ± 0.096
utils/index_constant_nodes/break_sharing 25.6 ± 1.6 μs 27.5 ± 1 μs 0.93 ± 0.068
utils/index_constant_nodes/preserve_sharing 0.101 ± 0.0052 ms 0.101 ± 0.0056 ms 0.993 ± 0.075
utils/is_constant/break_sharing 4.23 ± 0.2 μs 4.07 ± 0.27 μs 1.04 ± 0.085
utils/simplify_tree/break_sharing 0.169 ± 0.0051 ms 24.4 ± 0.67 μs 6.91 ± 0.28
utils/simplify_tree/preserve_sharing 0.22 ± 0.0096 ms 0.107 ± 0.0057 ms 2.05 ± 0.14
utils/string_tree/break_sharing 0.477 ± 0.023 ms 0.482 ± 0.019 ms 0.989 ± 0.061
utils/string_tree/preserve_sharing 0.595 ± 0.029 ms 0.588 ± 0.026 ms 1.01 ± 0.067
time_to_load 0.219 ± 0.0067 s 0.226 ± 0.0094 s 0.971 ± 0.05

Comment on lines +292 to +297
_children = if !isnothing(l) && isnothing(r)
@assert isnothing(children)
(l,)
elseif !isnothing(l) && !isnothing(r)
@assert isnothing(children)
(l, r)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add depwarn here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants