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 64 commits into
base: master
Choose a base branch
from
Open

Permit n-argument operators #127

wants to merge 64 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 2bf20fa... master / 2bf20fa...
eval/ComplexF32/evaluation 7.24 ± 0.53 ms 7.33 ± 0.6 ms 0.988 ± 0.11
eval/ComplexF64/evaluation 10.8 ± 0.81 ms 10.9 ± 1.1 ms 0.99 ± 0.13
eval/Float32/derivative 11.7 ± 0.59 ms 12.3 ± 0.75 ms 0.953 ± 0.076
eval/Float32/derivative_turbo 11.5 ± 0.53 ms 12.3 ± 0.78 ms 0.938 ± 0.074
eval/Float32/evaluation 2.69 ± 0.28 ms 2.71 ± 0.27 ms 0.992 ± 0.14
eval/Float32/evaluation_bumper 0.599 ± 0.015 ms 0.614 ± 0.016 ms 0.976 ± 0.035
eval/Float32/evaluation_turbo 0.514 ± 0.027 ms 0.533 ± 0.025 ms 0.965 ± 0.069
eval/Float32/evaluation_turbo_bumper 0.601 ± 0.015 ms 0.612 ± 0.015 ms 0.983 ± 0.035
eval/Float64/derivative 14.7 ± 1.2 ms 15.5 ± 1.7 ms 0.948 ± 0.13
eval/Float64/derivative_turbo 14.8 ± 1.3 ms 15.3 ± 1.6 ms 0.965 ± 0.13
eval/Float64/evaluation 3.12 ± 0.34 ms 3.16 ± 0.34 ms 0.987 ± 0.15
eval/Float64/evaluation_bumper 1.25 ± 0.042 ms 1.27 ± 0.045 ms 0.987 ± 0.048
eval/Float64/evaluation_turbo 1.01 ± 0.067 ms 1.04 ± 0.062 ms 0.969 ± 0.087
eval/Float64/evaluation_turbo_bumper 1.25 ± 0.041 ms 1.27 ± 0.043 ms 0.982 ± 0.046
utils/combine_operators/break_sharing 0.0395 ± 0.00064 ms 0.0387 ± 0.00075 ms 1.02 ± 0.026
utils/convert/break_sharing 26.8 ± 3.2 μs 26 ± 3 μs 1.03 ± 0.17
utils/convert/preserve_sharing 0.0983 ± 0.0047 ms 0.1 ± 0.0064 ms 0.981 ± 0.078
utils/copy/break_sharing 27.1 ± 3.1 μs 25.7 ± 2.7 μs 1.05 ± 0.17
utils/copy/preserve_sharing 0.0983 ± 0.0045 ms 0.101 ± 0.0064 ms 0.974 ± 0.076
utils/count_constant_nodes/break_sharing 8.56 ± 0.15 μs 8.38 ± 0.17 μs 1.02 ± 0.028
utils/count_constant_nodes/preserve_sharing 0.0853 ± 0.0032 ms 0.0849 ± 0.0043 ms 1 ± 0.064
utils/count_depth/break_sharing 10.2 ± 0.21 μs 10.2 ± 1.4 μs 0.999 ± 0.14
utils/count_nodes/break_sharing 8.68 ± 0.16 μs 8.17 ± 0.24 μs 1.06 ± 0.037
utils/count_nodes/preserve_sharing 0.085 ± 0.0034 ms 0.0858 ± 0.0046 ms 0.99 ± 0.066
utils/get_set_constants!/break_sharing 0.0332 ± 0.0025 ms 0.0321 ± 0.002 ms 1.03 ± 0.1
utils/get_set_constants!/preserve_sharing 0.179 ± 0.0078 ms 0.177 ± 0.0085 ms 1.01 ± 0.065
utils/get_set_constants_parametric 0.044 ± 0.0023 ms 0.0446 ± 0.0025 ms 0.987 ± 0.076
utils/has_constants/break_sharing 4.35 ± 0.12 μs 4.59 ± 0.19 μs 0.949 ± 0.047
utils/has_operators/break_sharing 2.32 ± 0.057 μs 2.32 ± 0.092 μs 1 ± 0.047
utils/hash/break_sharing 23.1 ± 0.5 μs 21.9 ± 0.79 μs 1.06 ± 0.045
utils/hash/preserve_sharing 0.0981 ± 0.0041 ms 0.1 ± 0.0058 ms 0.976 ± 0.07
utils/index_constant_nodes/break_sharing 25 ± 1.4 μs 27.4 ± 1.5 μs 0.912 ± 0.072
utils/index_constant_nodes/preserve_sharing 0.102 ± 0.0048 ms 0.104 ± 0.0064 ms 0.981 ± 0.076
utils/is_constant/break_sharing 3.95 ± 0.13 μs 3.87 ± 0.19 μs 1.02 ± 0.062
utils/simplify_tree/break_sharing 0.164 ± 0.0038 ms 24.6 ± 0.92 μs 6.7 ± 0.3
utils/simplify_tree/preserve_sharing 0.22 ± 0.0094 ms 0.107 ± 0.0058 ms 2.05 ± 0.14
utils/string_tree/break_sharing 0.469 ± 0.02 ms 0.487 ± 0.018 ms 0.962 ± 0.054
utils/string_tree/preserve_sharing 0.572 ± 0.02 ms 0.597 ± 0.021 ms 0.959 ± 0.047
time_to_load 0.218 ± 0.0046 s 0.244 ± 0.013 s 0.893 ± 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