local and collective contains for map, bag, counting set, and set

[sbromberger]

Includes tests.

Closes #393.

[steiltre]

This looks nice.

I notice the local_contains on the counting_set does not check the cache of counted keys. This behavior seems right to me (and does not affect results when calling the collective contains method), but may be something we change in the future.

[sbromberger]

I notice the local_contains on the counting_set does not check the cache of counted keys.

Can you explain more?

[steiltre]

With a counting_set, each rank holds part of a ygm::container::map that holds the counts, as well as a local cache of the recent items seen that allows us to hold on to frequently-seen items before sending them to their final location in the map.

The local_contains operation checks the ygm::container::map, but it doesn't check the local cache. When using the collective contains method, this does not matter because the local cache is going to be flushed during the barrier that precedes the reduction.

In the case that local_contains is being called by a user, there is a slight difference. The way it is implemented, a rank will only check the items that it is responsible for holding when deciding if a key is present (which is the correct decision in my mind). The alternative of checking the cache as well would return true for local_contains if the key was not owned by the current rank, but the rank happened to have a count stored locally in its cache.

Looking at it slightly more closely, there is a slight issue with that can come up from a key inserted locally that is actually owned by the rank calling async_insert could be trapped in the cache and not get seen by a call to local_contains. Skipping the cache and inserting the value directly into the ygm::container::map would not solve the issue in this case because messages from a rank to itself are still buffered (and not buffering them has caused issues in the past). To be more explicit, this code

my_counting_set.async_insert(local_key);
bool flag = my_counting_set.local_contains(local_key);

can result in flag being false. Given the semantics of async_ operations, this behavior is correct, but it might be unexpected for a savvy user who thinks they're being clever.

The choice to not check the cache in local_contains is the correct choice. I was simply making a note of it for when something breaks in the future.

[sbromberger]

Gotcha. Yeah, I also think this is reasonable. I'm assuming based on what you've said that a barrier between the async_insert and the local_contains resolves this issue? If so, this is consistent with other local operations - if there are pending messages from other ranks (and this is just a special case where the sending rank = the receiving rank), then they won't get captured until the barrier.

(Do I have it generally right?)

[steiltre]

You are correct. Barriers between the calls makes everything behave as expected, and the behavior is consistent regardless of whether the sender and receiver are the same rank. The only issue comes from misusing local_ operations.

[sbromberger]

Does it make sense to put a barrier as the first thing in a local_ function or do we just assume that the user understands the nuance?

[steiltre]

A barrier() won't work inside of local_ functions as they are not meant to be collectives. Users have to know what they are doing when using them.