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

*nodes return reduced dimension arrays/views whereas *spacings returns KernelFunctionOperators #3927

Open
ali-ramadhan opened this issue Nov 15, 2024 · 1 comment

Comments

@ali-ramadhan
Copy link
Member

Following the merging of #3143 the *spacings functions return a KernelFunctionOperator.

This is great, but should we be worried that this is inconsistent with the *nodes functions which return reduced dimension arrays/views?

If *nodes also returned KernelFunctionOperators would this make for a more consistent and powerful user interface?


julia> grid = RectilinearGrid(topology = (Periodic, Periodic, Bounded),
                              size = (4, 5, 4),
                              halo = (1, 1, 1),
                              x = (0, 1),
                              y = (0, 1),
                              z = [0, 0.1, 0.3, 0.6, 1])
4×5×4 RectilinearGrid{Float64, Periodic, Periodic, Bounded} on CPU with 1×1×1 halo
├── Periodic x  [0.0, 1.0) regularly spaced with Δx=0.25
├── Periodic y  [0.0, 1.0) regularly spaced with Δy=0.2
└── Bounded  z  [0.0, 1.0] variably spaced with min(Δz)=0.1, max(Δz)=0.4

julia> znodes(grid, Face())
5-element view(OffsetArray(::Vector{Float64}, 0:6), 1:5) with eltype Float64:
 0.0
 0.1
 0.3
 0.6
 1.0

julia> zspacings(grid, Face())
KernelFunctionOperation at (, , Face)
├── grid: 4×5×4 RectilinearGrid{Float64, Periodic, Periodic, Bounded} on CPU with 1×1×1 halo
├── kernel_function: zspacing (generic function with 27 methods)
└── arguments: ("Nothing", "Nothing", "Face")
@glwagner
Copy link
Member

I think so, because then coordinates can participate in AbstractOperations.

I noticed that PartialCellBottom does not extend znode. But perhaps it should. In that case, KernelFunctionOperation is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants