ygm::container common interface discussion thread

[rogerpearce]

I wanted to start a thread to discuss the common interface amongst ygm::containers, and figured we should test out the newish GitHub discussion features. What are your thoughts here?

Common API across all YGM containers

• using for_all_args = std::tuple< /separated args for for_all/ >
• for_all(Fn)
• for_all_consume(Fn)
• remove_if(Fn)
• size()
• clear()
• swap()
• get_ygm_ptr()
• comm()

set and sticky_set common API

• async_insert
• async_exe_if_contains
• async_exe_if_missing
• async_insert_exe_if_contains
• async_insert_exe_if_missing

bag

• async_insert

tagged_bag

• async_insert (returns unique opaque tag)
• async_visit(tag) -- ignores all previously deleted tags.
• async_erase(tag)

map and multimap

• async_insert
• async_set
• async_insert_if_missing
• async_visit_if_exists
• async_insert_or_reduce
• async_insert_if_missing_else_visit

[bwpriest]

What is for_all_consume(Fn) meant to do? I don't recall having discussed this pattern.

[rogerpearce]

It 'consumes' (erases) the elements as it loops over them. This helps with pipeline type algorithms that want to keep memory usage down.

[bwpriest]

We've also spoken about a lot of adapters for applying lambdas over only keys or only values, or splitting an std::pair value type up into different lambda arguments. Are those API elements meant to exist external to the container API and be overlaid? e.g. one would do something like ygm::value_only_adapter(my_map).for_all([](value_type &value) { /* do something */});?

[rogerpearce]

This is what I was envisioning.

[bwpriest]

* async_exe_if_contains

* async_exe_if_missing

* async_insert_exe_if_contains

* async_insert_exe_if_missing

I don't understand the rationale of using exe here instead of visit, as is used elsewhere in the codebase. Is there something fundamentally different between ygm::set::async_exe_if_contains and ygm::map::async_visit_if_exists? It seems to me that these functions should have the same names.

[rogerpearce]

'exe' is specific to set for a couple reasons:

• the keys in set are immutable, so the 'visit' wouldn't be something that can edit the key
• For sets where we only allow insertion (which is an option I want to add), we can use caching such that we can 'exe' without actually moving the lambda to the hashed owner location.
• The main use case is to be used as a conditional, "if key exists, execute this unrelated lambda"

[bwpriest]

That makes sense, thanks

[rogerpearce]

@bwpriest @steiltre

I'm now reconsidering having a local accessor method for map & array for locally owned keys. This would help higher-level data structures that know they have multiple ygm::containers sharing the same partitioning. What do you think about adding operator[](const key&) that throws an exception if the key isn't owned by the calling rank?

[steiltre]

Do you remember the reason we had to get rid of the shortcut for messages to self in ygm::comm? I think it was explicitly about recursion involving self-messages, but I don't remember exactly.

Allowing containers local access when they know they are partitioned the same way was a goal of ours. Just want to make sure we don't reintroduce a bug.

[rogerpearce]

It was related to modifying the container (and then its iterators) with either an insert/delete while also for_all looping over it (or visiting with multiple duplicate keys in multimap). We could not allow insert/removal of keys in a map, and return an std::optional<value_type&>. Optional-false if either the key doesn't exist, or if its mapped to another rank.

[bwpriest]

The behavior that you are describing is more similar to std::map::at than std::map::operator[], the latter of which creates an new key-value pair when an unset key gets passed. Would it make sense to use ygm::array::at and ygm::map::at, or would we rather use the accessor operator for aesthetic reasons?

[rogerpearce]

Good call. (Actually, I didn't know that at() didn't create a key until you mentioned this). So, yes, at( key ) should be the name.

So there are two key missing cases to consider:

1. Key would be owned local, but does not exist.
2. Key is not partitioned locally and will never exist locally.

I propose for (1) we return an std::optional<std::reference_wrapper<T>> that is empty, and for (2) we actually throw an exception, call it ygm::out_of_partition or something similar.

Note that std::map::at( key ) throws std::out_of_range when the key is missing (case 1).