Fix computation work layout for faces along bounded dimensions

[ali-ramadhan]

Resolves #908

[glwagner]

No dispatch 😳

But no, looks good

[codecov]

Codecov Report

Merging #910 into master will increase coverage by 0.03%.
The diff coverage is 90.32%.

@@            Coverage Diff             @@
##           master     #910      +/-   ##
==========================================
+ Coverage   72.42%   72.46%   +0.03%     
==========================================
  Files         187      193       +6     
  Lines        5484     5688     +204     
==========================================
+ Hits         3972     4122     +150     
- Misses       1512     1566      +54     

Impacted Files	Coverage Δ
src/AbstractOperations/AbstractOperations.jl	33.33% <ø> (ø)
src/Oceananigans.jl	66.66% <ø> (ø)
src/Utils/Utils.jl	66.66% <50.00%> (-33.34%)	⬇️
src/Grids/Grids.jl	91.66% <80.00%> (+8.33%)	⬆️
src/Grids/grid_utils.jl	92.00% <80.00%> (+3.57%)	⬆️
src/AbstractOperations/computations.jl	71.79% <100.00%> (+0.74%)	⬆️
src/AbstractOperations/interpolation_utils.jl	71.42% <100.00%> (+8.09%)	⬆️
src/Utils/kernel_launching.jl	100.00% <100.00%> (ø)
test/test_abstract_operations.jl	99.36% <100.00%> (+0.05%)	⬆️
...lementations/anisotropic_biharmonic_diffusivity.jl	96.66% <0.00%> (-3.34%)	⬇️
... and 17 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0c2821...60fd8ea. Read the comment docs.

[glwagner]

Attack of the Booleans

[ali-ramadhan]

I thought it was clever. Definitely not the worst use of boolean math ever haha.

[glwagner]

Design could be a little cleaner but since it works its ok.

[glwagner]

Hmm by the way include_boundaries is a slight misnomer --- with include_boundaries=false we still include the left boundary, but not the right boundaries. Maybe we could use include_right_boundaries to be totally specific and clear.

As for the dispatch, I believe something similar to what total_length does would be good:

Oceananigans.jl/src/Grids/grid_utils.jl

Line 50 in 6d349e1

@inline total_length(::Type{Face}, ::Type{Bounded}, N, H=0) = N + 1 + 2H

would be nice to have an analogous functions length and size that don't include halos:

Base.length(loc, topo, N) = N
Base.length(::Type{Face}, ::Type{Bounded}, N) = N+1

function Base.size(loc, grid::AbstractGrid)
    N = (grid.Nx, grid.Ny, grid.Nz)
    return Tuple(length(loc[i], topology(grid, i), N[i]) for i = 1:3)
end

then if include_right_boundary=true, you get the worksize dimension from size(loc, grid).

(the shenanigans with Nx, Ny, Nz above shows why it's nice to use tuples for everything...)

(Dispatch on AbstractGrid may not be required)

Also as convenience don't forget

Base.size(loc, grid, i) = size(loc, grid)[i]

[ali-ramadhan]

Thanks for the suggestions! I think the code is cleaner now. Will make sure tests pass and git push.

[glwagner]

@ali-ramadhan ready to merge?