Remove major invalidations from ==

[ChrisRackauckas]

See #523 . Seems to only be used for tests according to the PR but it's a major compile time problem, so it doesn't make sense to keep them. This can be marked breaking if it breaks downstream, but it's rather easy to update downstream AD packages for this so the value proposition is pretty clear.

[codecov-commenter]

Codecov Report

Merging #524 (0e1159a) into main (a9840b4) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #524      +/-   ##
==========================================
- Coverage   93.03%   93.02%   -0.02%     
==========================================
  Files          15       15              
  Lines         862      860       -2     
==========================================
- Hits          802      800       -2     
  Misses         60       60              

Impacted Files	Coverage Δ
src/tangent_types/thunks.jl	95.37% <ø> (-0.09%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9840b4...0e1159a. Read the comment docs.

[mcabbott]

This sounds great.

Are we at the stage with these tools where they can be added to the tests, to flag something which would introduce major issues?

(Also, maybe we should add CRTestUtils to the downstream tests of this package?)

[oxinabox]

This is probably not going to break anything real, even the tests should be using the function from CRTU.
In contrast, declaring this breaking would create a ton of downstream work.
So I would be happier not to say this is breaking.

Tim Holy and I did look at this at JuliaCon, and we concluded then that these invalidations were few and were low impact because of what was hit.
But we only looked at interactions with Base. So might not be true for some packages.
Or we could have been wrong for some other reason

Before we merge this can we

• benchmark something vaguely realistic as show that in the real world this makes a difference. (Measuring time, not just invalidations)

Then:

• bump the minor version
• make matching PR to CR.jl updating whatever test is breaking, and restricting version. So we can merge both at same time.

[oxinabox]

bump

[ChrisRackauckas]

benchmark something vaguely realistic as show that in the real world this makes a difference. (Measuring time, not just invalidations)

Look, doing this benchmark to really show the timing isn't something anyone has invested time to do in the last 4 months, but pretty much every compile time study flags this as a problem. So at this point, I think we should checkmark it at least as something to fix an easy false-positive if it did contribute nothing to times.

Locally, ChainRules tests ended up failing for an odd reason which I couldn't actually recreate in the REPL

using Test, ChainRulesCore, ChainRulesTestUtils
using ChainRules
using FiniteDifferences
using LinearAlgebra
using LinearAlgebra.BLAS
using LinearAlgebra: dot
using Random
using SparseArrays
using StaticArrays
using Statistics
using Test

T = Float64
test_frule(identity, randn(T))
test_frule(identity, randn(T, 4))
test_frule(identity, Tuple(randn(T, 3)))

test_rrule(identity, randn(T))
test_rrule(identity, randn(T, 4))
test_rrule(identity, Tuple(randn(T, 3)))

T = ComplexF64
test_frule(identity, randn(T))
test_frule(identity, randn(T, 4))
test_frule(identity, Tuple(randn(T, 3)))

test_rrule(identity, randn(T))
test_rrule(identity, randn(T, 4))
test_rrule(identity, Tuple(randn(T, 3)))

So @oxinabox I think this is ready to just rip off the bandaid. What's going on in the ChainRules tests can be figured out separately, but I don't think that should block this.

[oxinabox]

Downstream Error

It is definitely this PR causing ChainRules.jl to break.
It's main branch runs fine locally, (as well as on it's nightly CI branch).

I don't think we can merge this, and thus lose all ability to run CI on ChainRules.jl til we debug that.

The error is:

  MethodError: test_approx(::Tangent{Tuple{Float64, Float64, Float64}, Tuple{Float64, Float64, Float64}}, ::Thunk{ChainRulesTestUtils.var"#53#57"{Tangent{Tuple{Float64, Float64, Float64}, Tuple{Float64, Float64, Float64}}}}, ::String) is ambiguous. Candidates:
    test_approx(actual, expected::AbstractThunk, msg; kwargs...) in ChainRulesTestUtils at /home/oxinabox/.julia/packages/ChainRulesTestUtils/fCvaU/src/check_result.jl:28
    test_approx(actual::Tangent{P, T}, expected, msg; kwargs...) where {T, P} in ChainRulesTestUtils at /home/oxinabox/.julia/packages/ChainRulesTestUtils/fCvaU/src/check_result.jl:108
  Possible fix, define
    test_approx(::Tangent{P, T}, ::AbstractThunk, ::Any) where {T, P}

This is I think caused by this change in this PR.
Because test_approx would only try and hit this method if ==(::Tangent{P, T}, ::AbstractThunk) returned false.
Which this PR causes.
But we can fix this in ChainRulesTestUtils fairly simply.
By defining

test_approx(actual::Tangent, expected::AbstractThunk, msg::Any) = test_approx(actual, unthunk(expected), msg)-

Which would unblock this PR.

---

benchmarks:

Benchmarking this took me like a minute of actual work and a couple of minutes of waiting for DifferentialEquations to finish precompiling.
It is pretty conclusively in-favor of this change.
(Though a crevat is that i did forget to update my registry before doing this so packages are a bit out of date; still that shouldn't change the validity of the result.)

Special Functions

CRC@1.13.0

julia> @time using SpecialFunctions;
 0.376541 seconds (718.99 k allocations: 44.210 MiB, 80.86% compilation time)

CRC#invalidations

julia> @time using SpecialFunctions;
  0.366335 seconds (719.00 k allocations: 44.213 MiB, 2.08% gc time, 77.52% compilation time)

DifferentialEquations

CRC@1.13.0

julia> @time using DifferentialEquations
 7.408940 seconds (19.70 M allocations: 1.483 GiB, 3.67% gc time, 14.54% compilation time)

CRC#invalidations

julia> @time using DifferentialEquations
  5.513351 seconds (13.71 M allocations: 1.117 GiB, 5.45% gc time, 9.62% compilation time)

---

I have about 30 minutes left to me today, so I will see about making the change to unblock this in CRTU

[ChrisRackauckas]

Diffractor failure is on master #552 (comment) and unrelated.

Merging.