JuliaLang / Compat.jl

Compatibility across Julia versions
Other
145 stars 117 forks source link

Strides for Adjoint & Transpose #710

Closed mcabbott closed 4 years ago

mcabbott commented 4 years ago

This adds https://github.com/JuliaLang/julia/pull/35929 which updated https://github.com/JuliaLang/julia/pull/29135 .

Edit: the more recently merged PR https://github.com/JuliaLang/julia/pull/35929 is marked "backport 1.5". I presume that this means the VERSION cutoff used here may need to change.

martinholters commented 4 years ago

I'll review later, but on first glance: Needs a README entry.

mcabbott commented 4 years ago

Sure. If I do it after #709 then I won't have to fix clashes.

This one may need to wait for the backport anyway.

martinholters commented 4 years ago

LGTM. Once the backport is done and the version conditional updated accordingly, this should be good to go.

martinholters commented 4 years ago

Travis Linux Julia 1.0:

generalized dot #32739: Test Failed at /home/travis/build/JuliaLang/Compat.jl/test/runtests.jl:173

  Expression: dot(x, transpose(A), y) ≈ dot(x, transpose(A) * y) ≈ x' * transpose(A) * y ≈ (x' * transpose(A)) * y

   Evaluated: 0.0016273856f0 ≈ 0.0016267318f0 ≈ 0.0016271947f0 ≈ 0.0016271947f0

AV Julia latest (1.6.0-DEV.452) 32 bit:

generalized dot #32739: Test Failed at C:\projects\compat-jl\test\runtests.jl:171
  Expression: dot(x, A, y) ≈ dot(A' * x, y) ≈ x' * A * y ≈ (x' * A) * y
   Evaluated: 0.0003067404f0 ≈ 0.00030654308f0 ≈ 0.00030654308f0 ≈ 0.00030654308f0

All others pass. Does this influence the dispatch of dot in some way here? E.g., going through BLAS more often than before or something?

The inputs here are randomly generated, that might explain why it only fails sometimes, but I haven't seen this before, so most likely related to this PR.

martinholters commented 4 years ago

On second thought, this PR shouldn't change anything on Julia 1.6.0-DEV.452 (except for adding tests, which are not the ones failing here), so hard to blame it. But the same change in Base could lead to same problem in the here, of course.

mcabbott commented 4 years ago

I don't understand this at all. It's both 1.0 on Travis and master on Appveyor, on the same test. Both seem like they ought to have been identical on earlier runs of this PR.

And I can't see how strides would change dot, rand(2,2)' isa StridedArray remains false so it shouldn't dispatch differently.

martinholters commented 4 years ago

Running

@testset begin
    n = 10
    for iter in 1:1000000
        A = randn(Float32, n, n)
        x = rand(Float32, n)
        y = rand(Float32, n)
        @test dot(x, transpose(A), y) ≈ dot(x, transpose(A)*y) ≈ *(x', transpose(A), y) ≈ (x'*transpose(A))*y
    end
end

gives around 200 fails across different Julia versions. Seems we were just very unlucky with the latest CI run. I'll restart and if it looks better this time, this should be ready to merge. The flaky test is an orthogonal issue.