(BREAKING) Refactor code to be more idiomatic

MilesCranmer commented 4 months ago

This is BREAKING because we are changing the .val field to be ::T rather than ::Union{T,Nothing}, rather than nothing as was done previously. Union types are not allowed in static compiled code, hence this change (long-term goal is to make this statically compilable).

There is now a single constructors for Nodes, using keywords:

Node{ComplexF64}(val=0.1)   # Create a complex-valued leaf with 0.1
Node(feature=1)   # Create a leaf with feature 1 (default type is Float32)
Node(op=1, l=Node(val=0.1))  # unaop[1](0.1)
Node(op=3, l=Node(val=-0.9), r=Node(feature=3))  # binop[3](-0.9, x3)
Node(op=3, children=(Node(val=-0.9), Node(feature=3)))  # Same thing

We also move all the string stuff to Strings.jl.

github-actions[bot] commented 4 months ago

Benchmark Results

	master	739788508086d4...	t[master]/t[739788508086d4...]
eval/ComplexF32/evaluation	7.38 ± 0.49 ms	7.45 ± 0.46 ms	0.99
eval/ComplexF64/evaluation	9.67 ± 0.72 ms	9.65 ± 0.77 ms	1
eval/Float32/derivative	10.8 ± 1.7 ms	10.7 ± 1.7 ms	1
eval/Float32/derivative_turbo	10.7 ± 1.7 ms	10.8 ± 1.8 ms	0.993
eval/Float32/evaluation	2.71 ± 0.22 ms	2.73 ± 0.23 ms	0.992
eval/Float32/evaluation_bumper	0.591 ± 0.012 ms	0.529 ± 0.013 ms	1.12
eval/Float32/evaluation_turbo	0.694 ± 0.029 ms	0.7 ± 0.029 ms	0.991
eval/Float32/evaluation_turbo_bumper	0.591 ± 0.012 ms	0.528 ± 0.013 ms	1.12
eval/Float64/derivative	14.1 ± 0.57 ms	14.1 ± 0.59 ms	1.01
eval/Float64/derivative_turbo	14.2 ± 0.68 ms	14.1 ± 0.61 ms	1.01
eval/Float64/evaluation	2.96 ± 0.23 ms	2.89 ± 0.23 ms	1.03
eval/Float64/evaluation_bumper	1.26 ± 0.042 ms	1.2 ± 0.043 ms	1.05
eval/Float64/evaluation_turbo	1.17 ± 0.06 ms	1.18 ± 0.057 ms	0.996
eval/Float64/evaluation_turbo_bumper	1.28 ± 0.042 ms	1.19 ± 0.043 ms	1.07
utils/combine_operators/break_sharing	0.0499 ± 0.0029 ms	0.041 ± 0.0012 ms	1.22
utils/convert/break_sharing	28 ± 1 μs	28.4 ± 1.1 μs	0.988
utils/convert/preserve_sharing	0.126 ± 0.0026 ms	0.128 ± 0.0034 ms	0.985
utils/copy/break_sharing	28.6 ± 1.1 μs	29 ± 0.93 μs	0.984
utils/copy/preserve_sharing	0.127 ± 0.0025 ms	0.127 ± 0.0027 ms	0.995
utils/count_constants/break_sharing	10.4 ± 0.16 μs	10.5 ± 0.15 μs	0.997
utils/count_constants/preserve_sharing	0.112 ± 0.0023 ms	0.112 ± 0.0022 ms	1
utils/count_depth/break_sharing	12.6 ± 0.2 μs	12.7 ± 0.21 μs	0.99
utils/count_nodes/break_sharing	10.1 ± 0.15 μs	10.2 ± 0.15 μs	0.989
utils/count_nodes/preserve_sharing	0.115 ± 0.0022 ms	0.115 ± 0.0025 ms	1
utils/get_set_constants!/break_sharing	0.0541 ± 0.00081 ms	0.0523 ± 0.00077 ms	1.03
utils/get_set_constants!/preserve_sharing	0.32 ± 0.006 ms	0.324 ± 0.0063 ms	0.99
utils/has_constants/break_sharing	4.28 ± 0.21 μs	4.87 ± 0.22 μs	0.878
utils/has_operators/break_sharing	1.77 ± 0.023 μs	1.77 ± 0.018 μs	0.999
utils/hash/break_sharing	29.8 ± 0.41 μs	29.7 ± 0.43 μs	1
utils/hash/preserve_sharing	0.132 ± 0.0024 ms	0.133 ± 0.0024 ms	0.999
utils/index_constants/break_sharing	27.4 ± 0.76 μs	27.6 ± 0.72 μs	0.992
utils/index_constants/preserve_sharing	0.127 ± 0.0025 ms	0.126 ± 0.0025 ms	1.01
utils/is_constant/break_sharing	4.77 ± 0.22 μs	4.86 ± 0.22 μs	0.98
utils/simplify_tree/break_sharing	0.18 ± 0.015 ms	0.174 ± 0.015 ms	1.03
utils/simplify_tree/preserve_sharing	0.319 ± 0.017 ms	0.311 ± 0.016 ms	1.03
utils/string_tree/break_sharing	0.504 ± 0.012 ms	0.499 ± 0.013 ms	1.01
utils/string_tree/preserve_sharing	0.646 ± 0.017 ms	0.648 ± 0.02 ms	0.997
time_to_load	0.172 ± 0.0029 s	0.189 ± 0.0019 s	0.91

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 7995262531

Details

-7 of 216 (96.76%) changed or added relevant lines in 11 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+0.07%) to 94.681%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/OperatorEnumConstruction.jl	6	8	75.0%
src/Strings.jl	70	72	97.22%
src/Equation.jl	53	56	94.64%
<!--	Total:	209	216	96.76%	-->

Files with Coverage Reduction	New Missed Lines	%
src/Equation.jl	1	93.81%
<!--	Total:	1		-->

Totals
Change from base Build 7770039671:	0.07%
Covered Lines:	1620
Relevant Lines:	1711

SymbolicML / DynamicExpressions.jl