Skip to content

Fix gradients of parametric expressions #88

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

Merged
merged 5 commits into from
Jun 29, 2024

Conversation

MilesCranmer
Copy link
Member

Due to the conversion of ParametricExpression to Node within the parametric expression evaluation, the gradients with respect to constants in the expression were not being propagated. This fixes it with a new ChainRulesCore.rrule.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 9725012512

Details

  • 11 of 11 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 95.463%

Totals Coverage Status
Change from base Build 9653484196: 0.02%
Covered Lines: 2167
Relevant Lines: 2270

💛 - Coveralls

@MilesCranmer MilesCranmer merged commit ba0f841 into master Jun 29, 2024
17 checks passed
@MilesCranmer MilesCranmer deleted the fix-parametric-gradients branch June 29, 2024 15:21
Copy link
Contributor

github-actions bot commented Jun 29, 2024

Benchmark Results

master a2f5673... master/a2f567337d1d3a...
eval/ComplexF32/evaluation 7.51 ± 0.46 ms 7.44 ± 0.46 ms 1.01
eval/ComplexF64/evaluation 9.8 ± 0.74 ms 9.75 ± 0.7 ms 1.01
eval/Float32/derivative 11 ± 1.7 ms 10.8 ± 1.6 ms 1.02
eval/Float32/derivative_turbo 10.9 ± 1.8 ms 10.9 ± 1.6 ms 1
eval/Float32/evaluation 2.76 ± 0.22 ms 2.79 ± 0.21 ms 0.988
eval/Float32/evaluation_bumper 0.593 ± 0.013 ms 0.534 ± 0.014 ms 1.11
eval/Float32/evaluation_turbo 0.712 ± 0.032 ms 0.703 ± 0.03 ms 1.01
eval/Float32/evaluation_turbo_bumper 0.594 ± 0.013 ms 0.534 ± 0.014 ms 1.11
eval/Float64/derivative 14.1 ± 0.64 ms 14 ± 0.61 ms 1.01
eval/Float64/derivative_turbo 14.3 ± 0.65 ms 14.8 ± 1 ms 0.965
eval/Float64/evaluation 2.94 ± 0.24 ms 2.91 ± 0.23 ms 1.01
eval/Float64/evaluation_bumper 1.3 ± 0.044 ms 1.2 ± 0.043 ms 1.09
eval/Float64/evaluation_turbo 1.2 ± 0.062 ms 1.19 ± 0.06 ms 1.01
eval/Float64/evaluation_turbo_bumper 1.3 ± 0.046 ms 1.2 ± 0.046 ms 1.08
utils/combine_operators/break_sharing 0.0408 ± 0.0013 ms 0.0414 ± 0.0013 ms 0.988
utils/convert/break_sharing 28.5 ± 1 μs 29.1 ± 1.1 μs 0.979
utils/convert/preserve_sharing 0.129 ± 0.0028 ms 0.132 ± 0.0032 ms 0.98
utils/copy/break_sharing 29.5 ± 0.99 μs 29.4 ± 1 μs 1.01
utils/copy/preserve_sharing 0.128 ± 0.0028 ms 0.132 ± 0.0032 ms 0.974
utils/count_constants/break_sharing 10.6 ± 0.16 μs 11.7 ± 0.2 μs 0.902
utils/count_constants/preserve_sharing 0.113 ± 0.0024 ms 0.116 ± 0.0026 ms 0.981
utils/count_depth/break_sharing 12.5 ± 0.22 μs 12.8 ± 0.22 μs 0.98
utils/count_nodes/break_sharing 9.97 ± 0.16 μs 10.3 ± 0.16 μs 0.968
utils/count_nodes/preserve_sharing 0.115 ± 0.0025 ms 0.119 ± 0.0026 ms 0.969
utils/get_set_constants!/break_sharing 0.121 ± 0.0042 ms 0.122 ± 0.0043 ms 0.993
utils/get_set_constants!/preserve_sharing 0.323 ± 0.0082 ms 0.321 ± 0.0075 ms 1.01
utils/has_constants/break_sharing 4.72 ± 0.22 μs 4.38 ± 0.23 μs 1.08
utils/has_operators/break_sharing 1.6 ± 0.016 μs 1.78 ± 0.02 μs 0.9
utils/hash/break_sharing 0.0337 ± 0.00054 ms 0.033 ± 0.00048 ms 1.02
utils/hash/preserve_sharing 0.137 ± 0.0032 ms 0.137 ± 0.0027 ms 1
utils/index_constants/break_sharing 27.6 ± 0.71 μs 27.7 ± 0.67 μs 0.996
utils/index_constants/preserve_sharing 0.13 ± 0.0031 ms 0.131 ± 0.0028 ms 0.997
utils/is_constant/break_sharing 4.65 ± 0.22 μs 4.9 ± 0.22 μs 0.948
utils/simplify_tree/break_sharing 0.153 ± 0.015 ms 0.163 ± 0.016 ms 0.935
utils/simplify_tree/preserve_sharing 0.285 ± 0.016 ms 0.271 ± 0.017 ms 1.05
utils/string_tree/break_sharing 0.391 ± 0.013 ms 0.386 ± 0.011 ms 1.01
utils/string_tree/preserve_sharing 0.535 ± 0.017 ms 0.521 ± 0.014 ms 1.03
time_to_load 0.232 ± 0.0034 s 0.228 ± 0.0033 s 1.02

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