CliMA / ClimaCore.jl

CliMA model dycore
https://clima.github.io/ClimaCore.jl/dev
Apache License 2.0
85 stars 8 forks source link

`SphereDomain` does not support `Int`s #1468

Open Sbozzolo opened 12 months ago

Sbozzolo commented 12 months ago
import ClimaComms: context
import ClimaCore: Spaces, Domains, Meshes, Topologies, Geometry, Fields

radius = 1000

h_quad = Spaces.Quadratures.GLL{4}()
h_domain = Domains.SphereDomain(radius)
h_mesh = Meshes.EquiangularCubedSphere(h_domain, 6)
h_topology = Topologies.Topology2D(
            context(),
            h_mesh,
            Topologies.spacefillingcurve(h_mesh),
        )
h_space = Spaces.SpectralElementSpace2D(
            h_topology,
            h_quad
        )

Returns

ERROR: MethodError: no method matching legendre(::Type{Int64}, ::Int64, ::GaussQuadrature.EndPt)

Closest candidates are:
  legendre(::Type{T}, ::Integer, ::GaussQuadrature.EndPt) where T<:AbstractFloat
   @ GaussQuadrature ~/.julia/packages/GaussQuadrature/XPJQd/src/GaussQuadrature.jl:114
  legendre(::Any, ::Any)
   @ GaussQuadrature ~/.julia/packages/GaussQuadrature/XPJQd/src/GaussQuadrature.jl:126
  legendre(::Type{T}, ::Integer) where T<:AbstractFloat
   @ GaussQuadrature ~/.julia/packages/GaussQuadrature/XPJQd/src/GaussQuadrature.jl:114
  ...
simonbyrne commented 12 months ago

We should just call float on the argument, which will promote it to a float.

Sbozzolo commented 12 months ago

SphereDomain is defined as

struct SphereDomain{FT} <: AbstractDomain where {FT <: AbstractFloat}
    radius::FT
end

So, I am surprised we can pass Ints here.

simonbyrne commented 12 months ago

Oh, that's an interesting one. This isn't actually specifying what we want. It's actually parsing as

struct SphereDomain{FT} <: (AbstractDomain where {FT <: AbstractFloat})
    radius::FT
end

However since

julia> AbstractDomain where {FT<:AbstractFloat}
AbstractDomain

this is the same as simply saying

struct SphereDomain{FT} <: AbstractDomain
    radius::FT
end

What we actually want is

struct SphereDomain{FT<:AbstractFloat} <: AbstractDomain
    radius::FT
end
simonbyrne commented 12 months ago

@Sbozzolo care to make the change (and add a test?)

Sbozzolo commented 12 months ago
struct SphereDomain{FT<:AbstractFloat} <: AbstractDomain
    radius::FT
end

Do we want it <: AbstractFloat or <: Real?

Sbozzolo commented 12 months ago
struct SphereDomain{FT<:AbstractFloat} <: AbstractDomain
    radius::FT
end

Do we want it <: AbstractFloat or <: Real?

I guess <:AbstractFloat, given that the external package GaussQuadrature wants floats.

I'll convert Ints to Floats.

simonbyrne commented 12 months ago

I think <: AbstractFloat