Error when computing the masked `Integral` of a `Field` on an `ImmersedBoundaryGrid`

[tomchor]

I'd like to compute the integral of a Field on an ImmersedBoundaryGrid given a mask but keep getting an error. Here's a MWE:

using Oceananigans
using Oceananigans.Grids: znode
using Oceananigans.Fields: @compute

grid_base = RectilinearGrid(size = (4, 4, 4), extent = (1, 1, 1))
                            
bottom(x, y) = -1/2
grid = ImmersedBoundaryGrid(grid_base, GridFittedBottom(bottom))
model = NonhydrostaticModel(; grid)

mask_ccf(args...) = ifelse(znode(args..., Center(), Center(), Face()) > -0.25, 1, 0)
mask = Field(KernelFunctionOperation{Center, Center, Face}(mask_ccf, grid_base))

w2 = model.velocities.w^2
@compute W2 = Field(Integral(w2; mask))

Maybe there's a better way to define mask? The docs weren't very clear on what Integral expects for mask. I'm also a bit confused regarding the difference between condition and mask. Should I be using condition for this? Basically I'd like to calculate the integral using only a subset of the domain.

A side note given these questions is that I think there's some room from improvement in the docstring there.

[glwagner]

Can you post the error?

[tomchor]

Sorry, I completely forgot!

ERROR: LoadError: MethodError: Cannot `convert` an object of type Oceananigans.AbstractOperations.BinaryOperation{Center, Center, Face, typeof(+), Oceananigans.AbstractOperations.BinaryOperation{Center, Center, Face, typeof(+), Oceananigans.AbstractOperations.BinaryOperation{Center, Center, Face, typeof(+), Oceananigans.AbstractOperations.BinaryOperation{Center, Center, Face, typeof(+), Float64, Field{Center, Center, Face, KernelFunctionOperation{Center, Center, Face, RectilinearGrid{Float64, Periodic, Periodic, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64, Float64}, Float64, Float64, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, CPU}, Float64, typeof(mask_ccf), Tuple{}}, RectilinearGrid{Float64, Periodic, Periodic, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64, Float64}, Float64, Float64, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, CPU}, Tuple{Colon, Colon, Colon}, OffsetArrays.OffsetArray{Float64, 3, Array{Float64, 3}}, Float64, FieldBoundaryConditions{BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, Nothing, Nothing, BoundaryCondition{Oceananigans.BoundaryConditions.Flux, Nothing}}, Oceananigans.Fields.FieldStatus{Float64}, Oceananigans.Fields.FieldBoundaryBuffers{Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing}}, typeof(Oceananigans.Operators.identity3), typeof(Oceananigans.Operators.identity4), RectilinearGrid{Float64, Periodic, Periodic, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64, Float64}, Float64, Float64, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, CPU}, Float64}, Field{Center, Center, Face, KernelFunctionOperation{Center, Center, Face, RectilinearGrid{Float64, Periodic, Periodic, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64, Float64}, Float64, Float64, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, CPU}, Float64, typeof(mask_ccf), Tuple{}}, RectilinearGrid{Float64, Periodic, Periodic, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64, Float64}, Float64, Float64, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, CPU}, Tuple{Colon, Colon, Colon}, OffsetArrays.OffsetArray{Float64, 3, Array{Float64, 3}}, Float64, FieldBoundaryConditions{BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, Nothing, Nothing, BoundaryCondition{Oceananigans.BoundaryConditions.Flux, Nothing}}, Oceananigans.Fields.FieldStatus{Float64}, Oceananigans.Fields.FieldBoundaryBuffers{Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing}}, typeof(Oceananigans.Operators.identity5), typeof(Oceananigans.Operators.identity1), RectilinearGrid{Float64, Periodic, Periodic, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64, Float64}, Float64, Float64, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, CPU}, Float64}, Field{Center, Center, Face, KernelFunctionOperation{Center, Center, Face, RectilinearGrid{Float64, Periodic, Periodic, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64, Float64}, Float64, Float64, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, CPU}, Float64, typeof(mask_ccf), Tuple{}}, RectilinearGrid{Float64, Periodic, Periodic, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64, Float64}, Float64, Float64, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, CPU}, Tuple{Colon, Colon, Colon}, OffsetArrays.OffsetArray{Float64, 3, Array{Float64, 3}}, Float64, FieldBoundaryConditions{BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, Nothing, Nothing, BoundaryCondition{Oceananigans.BoundaryConditions.Flux, Nothing}}, Oceananigans.Fields.FieldStatus{Float64}, Oceananigans.Fields.FieldBoundaryBuffers{Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing}}, typeof(Oceananigans.Operators.identity2), typeof(Oceananigans.Operators.identity3), RectilinearGrid{Float64, Periodic, Periodic, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64, Float64}, Float64, Float64, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, CPU}, Float64}, Field{Center, Center, Face, KernelFunctionOperation{Center, Center, Face, RectilinearGrid{Float64, Periodic, Periodic, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64, Float64}, Float64, Float64, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, CPU}, Float64, typeof(mask_ccf), Tuple{}}, RectilinearGrid{Float64, Periodic, Periodic, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64, Float64}, Float64, Float64, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, CPU}, Tuple{Colon, Colon, Colon}, OffsetArrays.OffsetArray{Float64, 3, Array{Float64, 3}}, Float64, FieldBoundaryConditions{BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, BoundaryCondition{Oceananigans.BoundaryConditions.Periodic, Nothing}, Nothing, Nothing, BoundaryCondition{Oceananigans.BoundaryConditions.Flux, Nothing}}, Oceananigans.Fields.FieldStatus{Float64}, Oceananigans.Fields.FieldBoundaryBuffers{Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing}}, typeof(Oceananigans.Operators.identity4), typeof(Oceananigans.Operators.identity5), RectilinearGrid{Float64, Periodic, Periodic, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64, Float64}, Float64, Float64, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, CPU}, Float64} to an object of type Float64

Closest candidates are:
  convert(::Type{T}, ::T) where T<:Number
   @ Base number.jl:6
  convert(::Type{T}, ::Number) where T<:Number
   @ Base number.jl:7
  convert(::Type{T}, ::T) where T
   @ Base Base.jl:84
  ...

Stacktrace:
  [1] setindex!
    @ ./array.jl:1024 [inlined]
  [2] setindex!
    @ ./subarray.jl:331 [inlined]
  [3] _setindex!(::IndexCartesian, ::SubArray{…}, ::Oceananigans.AbstractOperations.BinaryOperation{…}, ::Int64, ::Int64, ::Int64)
    @ Base ./abstractarray.jl:1425
  [4] setindex!
    @ ./abstractarray.jl:1395 [inlined]
  [5] _mapreducedim!(f::typeof(identity), op::typeof(Base.add_sum), R::SubArray{…}, A::Oceananigans.AbstractOperations.ConditionalOperation{…})
    @ Base ./reducedim.jl:312
  [6] mapreducedim!(f::Function, op::Function, R::SubArray{…}, A::Oceananigans.AbstractOperations.ConditionalOperation{…})
    @ Base ./reducedim.jl:326
  [7] sum!(f::Function, r::SubArray{…}, A::Oceananigans.AbstractOperations.ConditionalOperation{…})
    @ Base ./reducedim.jl:1036
  [8] sum!(r::Field{…}, a::Oceananigans.AbstractOperations.ConditionalOperation{…}; condition::Nothing, mask::Int64, kwargs::@Kwargs{})
    @ Oceananigans.Fields /glade/work/tomasc/.julia/packages/Oceananigans/L7qIm/src/Fields/field.jl:689
  [9] sum!
    @ /glade/work/tomasc/.julia/packages/Oceananigans/L7qIm/src/Fields/field.jl:683 [inlined]
 [10] compute!
    @ /glade/work/tomasc/.julia/packages/Oceananigans/L7qIm/src/Fields/scans.jl:69 [inlined]
 [11] compute!(field::Field{…})
    @ Oceananigans.Fields /glade/work/tomasc/.julia/packages/Oceananigans/L7qIm/src/Fields/scans.jl:65
 [12] top-level scope
    @ /glade/derecho/scratch/tomasc/tokara-strait/simulations/mwe.jl:15
 [13] include(fname::String)
    @ Base.MainInclude ./client.jl:494
 [14] top-level scope
    @ REPL[1]:1

[glwagner]

I think we can change the nomenclature here. I agree it is confusing. condition filters the operation, and mask is the value that replaces the operation result when condition==true. So we should probably refer to condition as "mask", and mask as "mask_value` or something.

[glwagner]

you may want

using Oceananigans
using Oceananigans.Grids: znode
using Oceananigans.Fields: @compute

underlying_grid = RectilinearGrid(size = (4, 4, 4), extent = (1, 1, 1))
                            
bottom(x, y) = -1/2
grid = ImmersedBoundaryGrid(underlying_grid, GridFittedBottom(bottom))
model = NonhydrostaticModel(; grid)

condition(i, j, k, grid) = ifelse(znode(i, j, k, grid, Center(), Center(), Face()) > -0.25, 1, 0)

w2 = model.velocities.w^2
@compute W2 = Field(Integral(w2; condition))

[glwagner]

If we change the kwarg to mask I think we should also change ConditionalOperation to MaskedOperation

[glwagner]

Perhaps mask / fill_value + MaskedOperation. (The reason for fill_value is because efficient masking on GPU requires computing a "neutral value" for reductions that are invoked, but do not affect the result.)

[glwagner]

cc @navidcy

[tomchor]

you may want

using Oceananigans using Oceananigans.Grids: znode using Oceananigans.Fields: @compute

underlying_grid = RectilinearGrid(size = (4, 4, 4), extent = (1, 1, 1))

bottom(x, y) = -1/2 grid = ImmersedBoundaryGrid(underlying_grid, GridFittedBottom(bottom)) model = NonhydrostaticModel(; grid)

condition(i, j, k, grid) = ifelse(znode(i, j, k, grid, Center(), Center(), Face()) > -0.25, 1, 0)

w2 = model.velocities.w^2 @compute W2 = Field(Integral(w2; condition))

This example gives me this error:

ERROR: MethodError: no method matching condition(::Int64, ::Int64, ::Int64, ::ImmersedBoundaryGrid{…}, ::Oceananigans.AbstractOperations.ConditionalOperation{…})

Closest candidates are:
  condition(::Any, ::Any, ::Any, ::Any)
   @ Main REPL[8]:1

Stacktrace:
  [1] evaluate_condition
    @ /glade/work/tomasc/.julia/packages/Oceananigans/L7qIm/src/AbstractOperations/conditional_operations.jl:155 [inlined]
  [2] evaluate_condition
    @ /glade/work/tomasc/.julia/packages/Oceananigans/L7qIm/src/ImmersedBoundaries/immersed_reductions.jl:85 [inlined]
  [3] getindex
    @ /glade/work/tomasc/.julia/packages/Oceananigans/L7qIm/src/AbstractOperations/conditional_operations.jl:149 [inlined]
  [4] _getindex(::IndexCartesian, ::Oceananigans.AbstractOperations.ConditionalOperation{…}, ::Int64, ::Int64, ::Int64)
    @ Base ./abstractarray.jl:1340
  [5] getindex
    @ ./abstractarray.jl:1290 [inlined]
  [6] macro expansion
    @ ./reducedim.jl:310 [inlined]
  [7] macro expansion
    @ ./simdloop.jl:77 [inlined]
  [8] _mapreducedim!(f::typeof(identity), op::typeof(Base.add_sum), R::SubArray{…}, A::Oceananigans.AbstractOperations.ConditionalOperation{…})
    @ Base ./reducedim.jl:309
  [9] mapreducedim!(f::Function, op::Function, R::SubArray{…}, A::Oceananigans.AbstractOperations.ConditionalOperation{…})
    @ Base ./reducedim.jl:326
 [10] sum!(f::Function, r::SubArray{…}, A::Oceananigans.AbstractOperations.ConditionalOperation{…})
    @ Base ./reducedim.jl:1036
 [11] sum!(r::Field{…}, a::Oceananigans.AbstractOperations.ConditionalOperation{…}; condition::Nothing, mask::Int64, kwargs::@Kwargs{})
    @ Oceananigans.Fields /glade/work/tomasc/.julia/packages/Oceananigans/L7qIm/src/Fields/field.jl:689
 [12] sum!
    @ /glade/work/tomasc/.julia/packages/Oceananigans/L7qIm/src/Fields/field.jl:683 [inlined]
 [13] compute!
    @ /glade/work/tomasc/.julia/packages/Oceananigans/L7qIm/src/Fields/scans.jl:69 [inlined]
 [14] compute!(field::Field{…})
    @ Oceananigans.Fields /glade/work/tomasc/.julia/packages/Oceananigans/L7qIm/src/Fields/scans.jl:65
 [15] top-level scope
    @ REPL[10]:1

Not sure what the last argument there (ConditionalOperation) should be.

[glwagner]

It's just in the function signature -- you don't have to use it. Sorry about that, try this:

condition(i, j, k, grid, co) = ifelse(znode(i, j, k, grid, Center(), Center(), Face()) > -0.25, 1, 0)

[navidcy]

Yes, I was also thinking about the renaming of condition-mask potential confusion... I do like the kwarg name "condition" but I do think that usually people think of "mask" as being their condition.

Perhaps if we have kwargs condition and masked_value would avoid such confusions?

[navidcy]

The fact that the condition can be a function goes beyond what people usually think of as a mask in the, right?

[navidcy]

The docs weren't very clear on what Integral expects for mask.

I agree. I think the relevant information is in ConditionalOperation docstring

https://clima.github.io/OceananigansDocumentation/stable/appendix/library/#Oceananigans.AbstractOperations.ConditionalOperation-Tuple{Oceananigans.Fields.AbstractField}

We should probably link to this docstring in the various ReducedOperations (Average, Integral, ...)

[tomchor]

The docs weren't very clear on what Integral expects for mask.

I agree. I think the relevant information is in ConditionalOperation docstring

https://clima.github.io/OceananigansDocumentation/stable/appendix/library/#Oceananigans.AbstractOperations.ConditionalOperation-Tuple{Oceananigans.Fields.AbstractField}

We should probably link to this docstring in the various ReducedOperations (Average, Integral, ...)

A doctest in the docstring also wouldn't hurt.

[tomchor]

It's just in the function signature -- you don't have to use it. Sorry about that, try this:

condition(i, j, k, grid, co) = ifelse(znode(i, j, k, grid, Center(), Center(), Face()) > -0.25, 1, 0)

For the record, condition needs to return a boolean. So what worked was:

using Oceananigans
using Oceananigans.Grids: znode
using Oceananigans.Fields: @compute

underlying_grid = RectilinearGrid(size = (4, 4, 4), extent = (1, 1, 1))

bottom(x, y) = -1/2
grid = ImmersedBoundaryGrid(underlying_grid, GridFittedBottom(bottom))
model = NonhydrostaticModel(; grid)

condition(i, j, k, grid, co) = znode(i, j, k, grid, Center(), Center(), Face()) > -0.25

w2 = model.velocities.w^2
@compute W2 = Field(Integral(w2; condition))

[glwagner]

Ah sorry, I misinterpreted your code. Thanks for fixing it!

[glwagner]

Yes, I was also thinking about the renaming of condition-mask potential confusion... I do like the kwarg name "condition" but I do think that usually people think of "mask" as being their condition.

Perhaps if we have kwargs condition and masked_value would avoid such confusions?

I think that mask is more conventional, since it is also used for numpy's masked_array. It's also a nicely visual interpretation of what is happening. In other contexts the term is fill_value I think.

If you agree I'll make this change

[glwagner]

Had this convo with chatgpt about it if its of interest: https://chatgpt.com/share/6802bf58-83dc-8002-a0b0-2a1989f25b9d

[glwagner]

PS this also came up in the context of coupling on ClimaOcean, since there we need to introduce the concept of a "land mask" (which is 1 over land, 0 over ocean). The term "mask" comes up quite frequently, but unfortunately we apply it inconsistently throughout the ecosystem. I would like to move towards a consistent application of the term "mask".

I'm worried that using "mask_value" but not the term "mask" is halfway --- we invoke the concept of a "mask", but we don't actually define what the mask is if we keep condition.

[tomchor]

Yes, I was also thinking about the renaming of condition-mask potential confusion... I do like the kwarg name "condition" but I do think that usually people think of "mask" as being their condition. Perhaps if we have kwargs condition and masked_value would avoid such confusions?

I think that mask is more conventional, since it is also used for numpy's masked_array. It's also a nicely visual interpretation of what is happening. In other contexts the term is fill_value I think.

If you agree I'll make this change

I think this is a good change 👍

[navidcy]

So you are suggesting the change:

• condition -> mask
• mask -> fill_value

And in the new era after the changes above, mask is still (like condition is atm), either a function that returns a boolean or an array of Bools, right?

I like these suggestions!

[tomchor]

I think he is proposing:

• condition -> mask
• mask -> mask_value

no?

[navidcy]

You are right; I had misinterpreted.

Actually mask -> masked_value. And I actually like that even more.

[glwagner]

Sorry no, @navidcy said it right:

• condition -> mask
• mask -> fill_value

if chatgpt isn't lying to me then numpy defines masked_value as "the value to be masked". Whereas fill_value is the value that replaces the masked values.

It makes semantic sense to me as well

[tomchor]

You are right; I had misinterpreted.

Actually mask -> masked_value. And I actually like that even more.

I think I agree with @navidcy that I like mask -> mask_value more than fill_value. But both are clear improvement over what we currently have so I'm okay with either.

EDIT: I just realized that @navidcy wrote maskED_value.

Note that I'm not suggested maskED_value, which as you pointed out is the value to be masked. mask_value, however, is pretty clearly the value of the mask, which is the other keyword, so I think it makes the interface more intuitive.

[glwagner]

"mask_value" and "masked_value" are too similar and relatedly I feel "mask_value" is ambiguous.

[navidcy]

Sorry no, @navidcy said it right:

• condition -> mask
• mask -> fill_value

if chatgpt isn't lying to me then numpy defines masked_value as "the value to be masked". Whereas fill_value is the value that replaces the masked values.

It makes semantic sense to me as well

Given that we find the numpy nomenclature clear and intuitive, there is an extra advantage in going forward with the numpy nomenclature which is that lots of people are used to that.