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

column on grids are inconsistent and do not actually make views #1766

Open
charleskawczynski opened this issue May 31, 2024 · 1 comment
Open
Labels
bug Something isn't working

Comments

@charleskawczynski
Copy link
Member

#1487 fixed some issues, but column does not actually make a view, so the underlying DataLayout for a VIJFH-backed ExtrudedFiniteDifferenceSpace is still a VIJFH DataLayout.

The issue here is that column on a broadcasted object now contains a mixture of VF and VIJFH datalayouts. Memory access patterns for our metric terms are now "stuck" to those governed by VIJFH. i.e., we cannot make MArray-backed datalayouts.

This is a hard block, stopping #1763 because we cannot make an MArray-backed VIJFH (as that would be huge), and we cannot reconstruct ExtrudedFiniteDifferenceGrid on the GPU, since it is mutable.

What we have now seems like a bad design in that I would expect column to recurse all the way down a broadcasted object, and it doesn't. It's also unfortunate that this is blocking #1763.

The same is probably true for slab.

@charleskawczynski charleskawczynski added the bug Something isn't working label May 31, 2024
@charleskawczynski
Copy link
Member Author

Here's a reproducer that can be used to help see what's going on:

import ClimaCore, ClimaComms, CUDA
@isdefined(TU) || include(
    joinpath(pkgdir(ClimaCore), "test", "TestUtilities", "TestUtilities.jl"),
);
import .TestUtilities as TU;
using Test, StaticArrays, IntervalSets, LinearAlgebra, BenchmarkTools
import ClimaCore: Domains, Spaces, Fields, Operators
import ClimaCore.Domains: Geometry
import LazyBroadcast as LB

const CT3 = Geometry.Contravariant3Vector
const C12 = ClimaCore.Geometry.Covariant12Vector
const ᶠwinterp = Operators.WeightedInterpolateC2F(
    bottom = Operators.Extrapolate(),
    top = Operators.Extrapolate(),
);
FT = Float64;
cspace = TU.CenterExtrudedFiniteDifferenceSpace(FT; zelem = 25, helem = 10);
fspace = Spaces.FaceExtrudedFiniteDifferenceSpace(cspace);
ᶜx = fill((; uₕ = zero(C12{FT}), ρ = FT(0)), cspace);
ᶠx = fill((; ᶠuₕ³ = zero(CT3{FT})), fspace);
ᶜJ = Fields.local_geometry_field(ᶜx).J;
bc = LB.@lazy_broadcasted @. ᶠx.ᶠuₕ³ = ᶠwinterp(ᶜx.ρ * ᶜJ, CT3(ᶜx.uₕ));

bc_col = Fields.column(bc, 1,1,1)

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

No branches or pull requests

1 participant