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

add Field sum reproducer #2131

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add Field sum reproducer #2131

wants to merge 1 commit into from

Conversation

juliasloan25
Copy link
Member

In CliMA/ClimaLand.jl#970, I encountered that sum(f, field) != sum(f, parent(field)). I'm not sure if this is the expected behavior but it was surprising to me, so I wanted to open a reproducer for it.

@charleskawczynski do you have any insight?

@juliasloan25
Copy link
Member Author

The behavior of sum is described somewhat in this thread, but it would be good to have more complete documentation of the expected behavior

@charleskawczynski
Copy link
Member

Yes, I hope I can shine some light on this. Our sum by default uses a weighting by the jacobian. From ClimaCore:

function Base.sum(
    field::Union{Field, Base.Broadcast.Broadcasted{<:FieldStyle}},
    dev::ClimaComms.CUDADevice,
)
    context = ClimaComms.context(axes(field))
    localsum = mapreduce_cuda(identity, +, field, weighting = true)
    ClimaComms.allreduce!(context, parent(localsum), +)
    call_post_op_callback() && post_op_callback(localsum[], field, dev)
    return localsum[]
end

Basically, it's an integral

@charleskawczynski
Copy link
Member

That said, there is likely still inconsistencies with how we handle sum when slicing data, as mentioned in #943.

I think we need to assemble a list of requirements / expectations, and perhaps define integral in ClimaCore, and document expected behavior in different situations.

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

Successfully merging this pull request may close these issues.

2 participants