Deprecate shareRaw

[franzpoeschel]

Idea: Instead of rc.storeChunk(shareRaw(ptr), …), have users write rc.storeChunkRaw(ptr, …) which is a bit more transparent in what is happening. Partially resolves #1216 by introducing a clearer API that distinguishes between owned and non-owned resources.

TODO

• Documentation
• The diff on examples, tests and Python bindings shows the proposed API changes. Discuss if they are ok like this. (Especially the changes in determineDatatype)
• Maybe revert some usage changes, so the CI can test if shareRaw continues working
• Introduce an internal version of shareRaw, so every creation of a non-owning shared pointer goes through the same function. Ideally, in the end a shared pointer is either owning or has been created by that function.

[ax3l]

This looks good to me and as discussed, let's proceed.

Only thing I would add is another overload for contiguous containers to determineDatatype for symmetry with storeChunk.

[ax3l]

@franzpoeschel pls resolve the merge conflict :)

[franzpoeschel]

@franzpoeschel pls resolve the merge conflict :)

Done

[ax3l]

For determineDatatype, I tend to just use one API function name with overloads for pointers and cont. containers.
There is no lifetime question here and additional naming is not really needed, adding unnecessary API calls to learn.

What do you think?

[franzpoeschel]

The reason I did it like this is a slight inconsistency that we would get:

determineDatatype<std::vector<float>>(); // evaluates to Datatype::VECTOR_FLOAT
determineDatatype(std::vector<float>()); // evaluates to Datatype::FLOAT, interpreting the vector as a contiguous container

As long as we don't add the following overload, this would technically work, but still be extremely confusing imo:

template <typename T>
inline constexpr Datatype determineDatatype(T)
{
     // ignoring reference decaying for this example now
    return determineDatatype<T>();
}

I see the appeal of not introducing new API calls left and right, but we should be aware of the possible source for confusion.

[franzpoeschel]

I checked the PR again, and we could theoretically do the entire thing only with new overloads, instead of new API calls (even replace storeChunkRaw with storeChunk). So this is entirely a design decision:

• storeChunkRaw(): I like making it explicit that we are dealing with non-owned, raw memory
• determineDatatypeContiguous(): Avoids confusion, as described above
• determineDatatypeRaw(): Would then be consistent with the change above

[ax3l]

I have a slide tendency to overloads, simply because it's C++ pattern (vs. C).

[ax3l]

For this case, we could make it consistently evaluate to Datatype::VECTOR_FLOAT:

determineDatatype<std::vector<float>>();
determineDatatype(std::vector<float>());

and ask the user to call via ::value_type or .data() to get the fundamental type aka Datatype::FLOAT.

determineDatatype<std::vector<float>::value_type>();
determineDatatype(std::vector<float>().data());

I think I overdid the original detection support here for STL :)

If this changes existing behavior, then let us note it in NEWS.rst pls.

[franzpoeschel]

I would maybe take your suggestion, but make it a little bit stricter:

If you want a direct translation between C++ types and our openPMD::Datatype enum, then use determineDatatype<T>() without argument (exactly as it is before this PR).

We provide overloads for pointer types, i.e. determineDatatype<T>(std::shared_ptr<T>) (already exists) and determineDatatype<T>(T const *) (new).

If a user passes a std::vector or a std::array, we don't even start guessing what is meant, but throw a detailed compile-time error:

Error: Passed a contiguous container type to determineDatatype<>().                                 [10/1843]
These types are not directly supported due to colliding semantics.                                           
Assuming a vector object `std::vector<float> vec;`,                                                          
use one of the following alternatives:                                                                       
                                                                                                             
1) If what you want is a direct openPMD::Datatype equivalent of the container                                
   type, use:                                                                                                
   `determineDatatype<decltype(vec)>()`                                                                      
   OR                                                                                                        
   `determineDatatype<std::vector<float>>()`.                                                                
   The result will be `Datatype::VECTOR_FLOAT`.                                                              
2) If what you want is the openPMD::Datatype equivalent of the *contained type*,
   use the raw pointer overload by:
   `determineDatatype(vec.data())`
   This is the variant that you likely wish to use if intending to write data
   from the vector via `storeChunk()`, e.g. `storeChunk(vec, {0}, {10})`.

Corresponding implementation:

template <typename T_ContiguousContainer>
inline constexpr Datatype determineDatatype(T_ContiguousContainer &&)
{
    using T_ContiguousContainer_stripped =
        std::remove_reference_t<T_ContiguousContainer>;
    if constexpr (auxiliary::IsContiguousContainer_v<
                      T_ContiguousContainer_stripped>)
    {
        static_assert(auxiliary::dependent_false_v<T_ContiguousContainer>, R"(
Error: Passed a contiguous container type to determineDatatype<>().
These types are not directly supported due to colliding semantics.
Assuming a vector object `std::vector<float> vec;`,
use one of the following alternatives:

1) If what you want is a direct openPMD::Datatype equivalent of the container
   type, use:
   `determineDatatype<decltype(vec)>()`
   OR
   `determineDatatype<std::vector<float>>()`.
   The result will be `Datatype::VECTOR_FLOAT`.
2) If what you want is the openPMD::Datatype equivalent of the *contained type*,
   use the raw pointer overload by:
   `determineDatatype(vec.data())`
   This is the variant that you likely wish to use if intending to write data
   from the vector via `storeChunk()`, e.g. `storeChunk(vec, {0}, {10})`.
        )");
    }
    else
    {
        static_assert(auxiliary::dependent_false_v<T_ContiguousContainer>, R"(
Unknown datatype passed to determineDatatype<>().
For a direct translation from C++ type to the openPMD::Datatype enum, use:
`auto determineDatatype<T>() -> Datatype`.

For detecting the contained datatpye of a pointer type (shared or raw pointer),
use either of the following overloads:
`auto determineDatatype<T>(std::shared_ptr<T>) -> Datatype` or
`auto determineDatatype<T>(T const *) -> Datatype`.
        )")
    }
}

This would entirely avoid any API inconsistencies, without introducing unexpected behavior where we need to guess what the user wanted.

[ax3l]

Do we need a std::shared_ptr<T[]>, too?

[franzpoeschel]

Yeah, I forgot that in #1296. Should I open a separate PR or just add it into this one?

[franzpoeschel]

Eh, I updated it now