Skip to content

CompatHelper: bump compat for Zygote in [weakdeps] to 0.7, (keep existing compat) #119

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

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jan 5, 2025

This pull request changes the compat entry for the Zygote package from 0.6 to 0.6, 0.7.
This keeps the compat entries for earlier versions.

Note: I have not tested your package with this new compat entry.
It is your responsibility to make sure that your package tests pass before you merge this pull request.

Copy link
Contributor Author

github-actions bot commented Feb 28, 2025

Benchmark Results

master 4b22d2c... master / 4b22d2c...
eval/ComplexF32/evaluation 7.34 ± 0.55 ms 7.38 ± 0.58 ms 0.994 ± 0.11
eval/ComplexF64/evaluation 11 ± 0.97 ms 10.9 ± 1.2 ms 1.01 ± 0.14
eval/Float32/derivative 12.3 ± 0.85 ms 13 ± 0.92 ms 0.947 ± 0.094
eval/Float32/derivative_turbo 12.7 ± 0.84 ms 13.3 ± 1.1 ms 0.952 ± 0.1
eval/Float32/evaluation 2.75 ± 0.28 ms 2.72 ± 0.27 ms 1.01 ± 0.14
eval/Float32/evaluation_bumper 0.597 ± 0.016 ms 0.602 ± 0.016 ms 0.992 ± 0.038
eval/Float32/evaluation_turbo 0.531 ± 0.034 ms 0.538 ± 0.029 ms 0.988 ± 0.083
eval/Float32/evaluation_turbo_bumper 0.599 ± 0.016 ms 0.601 ± 0.016 ms 0.997 ± 0.038
eval/Float64/derivative 16.2 ± 1.7 ms 16.6 ± 2 ms 0.972 ± 0.15
eval/Float64/derivative_turbo 16.4 ± 1.9 ms 16.8 ± 2.1 ms 0.981 ± 0.16
eval/Float64/evaluation 3.19 ± 0.36 ms 3.2 ± 0.33 ms 0.998 ± 0.15
eval/Float64/evaluation_bumper 1.27 ± 0.046 ms 1.26 ± 0.046 ms 1.01 ± 0.051
eval/Float64/evaluation_turbo 1.04 ± 0.071 ms 1.05 ± 0.069 ms 0.996 ± 0.094
eval/Float64/evaluation_turbo_bumper 1.27 ± 0.045 ms 1.26 ± 0.045 ms 1.01 ± 0.051
utils/combine_operators/break_sharing 0.0393 ± 0.0011 ms 0.0391 ± 0.00078 ms 1 ± 0.034
utils/convert/break_sharing 28.5 ± 3.7 μs 28.5 ± 3.8 μs 0.999 ± 0.19
utils/convert/preserve_sharing 0.102 ± 0.0053 ms 0.102 ± 0.0064 ms 0.995 ± 0.081
utils/copy/break_sharing 29.1 ± 3.4 μs 28.9 ± 3.3 μs 1.01 ± 0.17
utils/copy/preserve_sharing 0.102 ± 0.0058 ms 0.101 ± 0.0065 ms 1.01 ± 0.087
utils/count_constant_nodes/break_sharing 9.01 ± 0.75 μs 9.35 ± 0.67 μs 0.964 ± 0.11
utils/count_constant_nodes/preserve_sharing 0.0876 ± 0.0047 ms 0.0874 ± 0.005 ms 1 ± 0.079
utils/count_depth/break_sharing 10.8 ± 0.96 μs 10.5 ± 1.2 μs 1.03 ± 0.15
utils/count_nodes/break_sharing 9.72 ± 0.66 μs 8.96 ± 0.88 μs 1.08 ± 0.13
utils/count_nodes/preserve_sharing 0.0881 ± 0.0051 ms 0.087 ± 0.0046 ms 1.01 ± 0.079
utils/get_set_constants!/break_sharing 0.0346 ± 0.003 ms 0.0347 ± 0.0022 ms 0.995 ± 0.11
utils/get_set_constants!/preserve_sharing 0.18 ± 0.011 ms 0.179 ± 0.0093 ms 1 ± 0.078
utils/get_set_constants_parametric 0.0463 ± 0.0029 ms 0.0465 ± 0.0032 ms 0.996 ± 0.093
utils/has_constants/break_sharing 4.78 ± 0.37 μs 4.47 ± 0.23 μs 1.07 ± 0.098
utils/has_operators/break_sharing 2.44 ± 0.18 μs 2.4 ± 0.15 μs 1.01 ± 0.099
utils/hash/break_sharing 23.2 ± 0.69 μs 22.8 ± 0.57 μs 1.02 ± 0.04
utils/hash/preserve_sharing 0.0991 ± 0.0045 ms 0.0978 ± 0.0043 ms 1.01 ± 0.064
utils/index_constant_nodes/break_sharing 27.2 ± 2.4 μs 27.1 ± 2.3 μs 1 ± 0.12
utils/index_constant_nodes/preserve_sharing 0.101 ± 0.0055 ms 0.101 ± 0.0063 ms 1 ± 0.083
utils/is_constant/break_sharing 4.21 ± 0.36 μs 4.32 ± 0.33 μs 0.974 ± 0.11
utils/simplify_tree/break_sharing 0.171 ± 0.0056 ms 0.17 ± 0.0056 ms 1.01 ± 0.047
utils/simplify_tree/preserve_sharing 0.224 ± 0.0093 ms 0.25 ± 0.011 ms 0.897 ± 0.053
utils/string_tree/break_sharing 0.496 ± 0.032 ms 0.508 ± 0.032 ms 0.976 ± 0.087
utils/string_tree/preserve_sharing 0.6 ± 0.035 ms 0.611 ± 0.037 ms 0.981 ± 0.082
time_to_load 0.23 ± 0.0036 s 0.24 ± 0.0029 s 0.957 ± 0.019

@asinghvi17
Copy link

Is there a plan to merge this or look at ongoing support for Zygote 0.7?

@MilesCranmer
Copy link
Member

Sorry, I just forgot to do this. Not sure what the CI errors were.. Let me try merging again

@MilesCranmer
Copy link
Member

MilesCranmer commented May 23, 2025

Hitting this method error

eachindex(::ChainRulesCore.InplaceableThunk{ChainRulesCore.Thunk{ChainRules.var"#719#722"{Float64, Colon, Vector{Float64}, ChainRulesCore.ProjectTo{AbstractArray, @NamedTuple{element::ChainRulesCore.ProjectTo{Float64, @NamedTuple{}}, axes::Tuple{Base.OneTo{Int64}}}}}}, ChainRules.var"#718#721"{Float64, Colon}}, ::Base.OneTo{Int64})

@MilesCranmer
Copy link
Member

MilesCranmer commented May 23, 2025

Pretty sure this is a Zygote.jl ChainRules bug

Nevermind. It just needed a manual unthunk call

@coveralls
Copy link

Pull Request Test Coverage Report for Build 15218831644

Details

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

Totals Coverage Status
Change from base Build 14729298079: 0.001%
Covered Lines: 2573
Relevant Lines: 2692

💛 - Coveralls

@MilesCranmer MilesCranmer merged commit 7540284 into master May 23, 2025
8 checks passed
@MilesCranmer MilesCranmer deleted the compathelper/new_version/2025-01-05-01-23-24-852-00256837307 branch May 23, 2025 21:25
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.

3 participants