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 failure when broadcasting over multiple structs and a Field #2065

Open
juliasloan25 opened this issue Oct 31, 2024 · 1 comment · May be fixed by #2071
Open

inference failure when broadcasting over multiple structs and a Field #2065

juliasloan25 opened this issue Oct 31, 2024 · 1 comment · May be fixed by #2071
Labels
bug Something isn't working

Comments

@juliasloan25
Copy link
Member

juliasloan25 commented Oct 31, 2024

Describe the bug

Reproducer added in #2062
Initially found in CliMA/ClimaLand.jl#739

This is a type inference error that arises when a function is broadcast over one field and two structs that both have broadcastable extended using tuple. The types in the expression become too complex, and inference fails. The problem is simplified in the reproducer so the structs each contain one scalar value, and the called function itself does nothing. The reproducer was tested on fields constructed on multiple different spaces, and reproduced the bug on all of them.

@juliasloan25 juliasloan25 added the bug Something isn't working label Oct 31, 2024
@charleskawczynski charleskawczynski linked a pull request Nov 1, 2024 that will close this issue
juliasloan25 added a commit to CliMA/ClimaLand.jl that referenced this issue Dec 11, 2024
This is required as a workaround to the type inference failure detailed
in CliMA/ClimaCore.jl#2065. The failure appears
when a function is broadcast over one field and two structs; the types
are too complex and inference fails. This workaround introduces a wrapper
struct to contain the two struct inputs, so that the function is broadcast
over one field and only one struct, and the types are easier to infer.
juliasloan25 added a commit to CliMA/ClimaLand.jl that referenced this issue Dec 11, 2024
This is required as a workaround to the type inference failure detailed
in CliMA/ClimaCore.jl#2065. The failure appears
when a function is broadcast over one field and two structs; the types
are too complex and inference fails. This workaround introduces a wrapper
struct to contain the two struct inputs, so that the function is broadcast
over one field and only one struct, and the types are easier to infer.
@charleskawczynski
Copy link
Member

charleskawczynski commented Jan 6, 2025

This is becoming more of an issue as we eliminate the ClimaAtmos solver cache. So, I think it's worth either fixing this or opening an issue in julialang.

Here is a pretty low-level reproducer, with only datalayouts (and arrays):

import ClimaCore: DataLayouts
import Base.Broadcast: broadcastable,
	BroadcastStyle,
	AbstractArrayStyle,
	Broadcasted,
	instantiate,
	broadcasted,
	materialize,
	materialize!

struct MyParams1{A}
  a::A
end;
struct MyParams2{B}
  b::B
end;
Base.Broadcast.broadcastable(x::MyParams1) = tuple(x);
Base.Broadcast.broadcastable(x::MyParams2) = tuple(x);

foo(f, p1, p2) = f + p1.a - p2.b;
bar(p1, p2, f) = f + p1.a - p2.b;

FT = Float64;
p1 = MyParams1{FT}(1);
p2 = MyParams2{FT}(2);

b = zeros(FT, 5,5); # works
a = similar(b);
bc = instantiate(broadcasted(foo, b, p1, p2));
materialize!(a, bc)
using JET; @test_opt materialize!(a, bc) # also passes inference

b = DataLayouts.VIJFH{FT}(Array{FT}, zeros; Nv=4, Nij=4, Nh=4); # ClimaCore CPU array works
a = similar(b);
bc = instantiate(broadcasted(foo, b, p1, p2));
materialize!(a, bc)
using JET; @test_opt materialize!(a, bc) # also passes inference

import CUDA;
b = CUDA.zeros(FT, 5,5); # cu-array works
a = similar(b);
bc = instantiate(broadcasted(foo, b, p1, p2));
materialize!(a, bc)

import CUDA;
b = DataLayouts.VIJFH{FT}(CUDA.CuArray{FT}, zeros; Nv=4, Nij=4, Nh=4);
a = similar(b);
bc = instantiate(broadcasted(foo, b, p1, p2));
materialize!(a, bc) # fails

nothing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants