Skip to content
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

inference problem with broadcasting and / #682

Closed
tpapp opened this issue Oct 29, 2019 · 6 comments · Fixed by #1079
Closed

inference problem with broadcasting and / #682

tpapp opened this issue Oct 29, 2019 · 6 comments · Fixed by #1079
Labels
broadcast fix-in-base Fix needs some work in Base performance runtime performance

Comments

@tpapp
Copy link
Contributor

tpapp commented Oct 29, 2019

This is an MWE isolated from a larger piece of code:

using ForwardDiff, StaticArrays, Test
_dual(x) = ForwardDiff.Dual(x, 1 + x) # for testing
f(η, B, R, 𝑢, σ, C) =.* (B .- R) ./ 𝑢 .- C) ./ σ
A = fill(1.0, SMatrix{2, 2})
b = 1.0
@inferred f(A, b, A, b, b, A)        # infers fine
@inferred f(A, b, A, _dual(b), b, A) # does not infer

Version information:

julia> versioninfo()
Julia Version 1.3.0-rc4.1
Commit 8c4656b97a (2019-10-15 14:08 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.1 (ORCJIT, skylake)
Environment:
  JULIA_CMDSTAN_HOME = /home/tamas/src/cmdstan/

(v1) pkg> st StaticArrays ForwardDiff
    Status `~/.julia/environments/v1/Project.toml`
  [163ba53b] DiffResults v0.0.4
  [f6369f11] ForwardDiff v0.10.3
  [276daf66] SpecialFunctions v0.7.2
  [90137ffa] StaticArrays v0.11.0 #master (https://github.com/JuliaArrays/StaticArrays.jl.git)
  [b77e0a4c] InteractiveUtils 
  [37e2e46d] LinearAlgebra 
@tpapp
Copy link
Contributor Author

tpapp commented Oct 29, 2019

And

f(η, B, R, 𝑢, σ, C) =.* (B .- R) / 𝑢 .- C) ./ σ

(note / before 𝑢) works fine.

@c42f c42f added the performance runtime performance label Nov 8, 2019
@c42f
Copy link
Member

c42f commented Nov 8, 2019

Thanks for the report. I do wonder whether there's anything we can realistically do about this, or whether a fix is required in Base. I guess a deep dive will be required to figure out what's going on.

@tpapp
Copy link
Contributor Author

tpapp commented Nov 12, 2019

I tried to investigate but could not figure it out, I was kind of hoping that someone here with more knowledge about debugging these issues would take it up.

@c42f
Copy link
Member

c42f commented Nov 14, 2019

Understood. I haven't had time to do the requisite digging and TBH Julia's type inference implementation is not something I know a lot about yet.

Any thoughts @martinholters? You've solved a few of these type of problems before :-)

@schmrlng
Copy link
Contributor

This might be related to #609 and JuliaLang/julia#32552 (and linked history within) -- this is definitely my biggest gripe with julia, that it's essentially impossible to write high performance code combining StaticArrays and broadcasting because type inference gets "tired" halfway through drilling down to the bottom of a huge stack of highly-parameterized Broadcasted types. Trying to make things easier for inference (below, asking it to do a bit less dot fusion) can be effective as a workaround but it's definitely kind of annoying:

  | | |_| | | | (_| |  |  Version 1.3.0-rc4.1 (2019-10-15)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> using ForwardDiff, StaticArrays, Test

julia> begin
           _dual(x) = ForwardDiff.Dual(x, 1 + x) # for testing
           broadcasted_division(x, y) = x ./ y
           f(η, B, R, 𝑢, σ, C) =.* broadcasted_division(B .- R, 𝑢) .- C) ./ σ
           A = fill(1.0, SMatrix{2, 2})
           b = 1.0
       end
1.0

julia> @inferred f(A, b, A, b, b, A)        # infers fine
2×2 SArray{Tuple{2,2},Float64,2,4} with indices SOneTo(2)×SOneTo(2):
 -1.0  -1.0
 -1.0  -1.0

julia> @inferred f(A, b, A, _dual(b), b, A) # now also infers fine
2×2 SArray{Tuple{2,2},ForwardDiff.Dual{Nothing,Float64,1},2,4} with indices SOneTo(2)×SOneTo(2):
 Dual{Nothing}(-1.0,-0.0)  Dual{Nothing}(-1.0,-0.0)
 Dual{Nothing}(-1.0,-0.0)  Dual{Nothing}(-1.0,-0.0)

(v1.3) pkg> st
    Status `~/.julia/environments/v1.3/Project.toml`
  [f6369f11] ForwardDiff v0.10.6
  [90137ffa] StaticArrays v0.12.1 #master (https://github.com/JuliaArrays/StaticArrays.jl.git)

@mateuszbaran
Copy link
Collaborator

Small update: I've proposed a solution here: JuliaLang/julia#41090 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broadcast fix-in-base Fix needs some work in Base performance runtime performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants