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

Document 'Collection' comparison with 'Container' named requirement #598

Merged
merged 43 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
d340640
Add document comparing collection with standard 'container' named req…
m-fila May 7, 2024
e1161e1
Add to collection: `cbegin`, `cend`, `max_size`, `size_type`, `differ…
m-fila May 7, 2024
d1539d4
Add basic tests for collection compliance with container named requi…
m-fila May 7, 2024
8e34c3d
add missing `max_size` implementation, add collection methods and ali…
m-fila May 7, 2024
3c74e1f
update documentation with container like methods and aliases
m-fila May 7, 2024
d7b1acc
Add placeholders for iterator requirements and interaction with std a…
m-fila May 7, 2024
e117c94
fix fail message
m-fila May 7, 2024
b1d59a8
check Erasable, fix consistency, add links to reference
m-fila May 8, 2024
8740945
fix consistency, add macro to indicate checks that may need updating …
m-fila May 8, 2024
0a98b44
add short note on AllocatorAwareContainer, simplify comments
m-fila May 8, 2024
a8af6ea
add singularity iterators check
m-fila May 8, 2024
18f9e71
rename pre-increment and post-increment
m-fila May 9, 2024
43a6b38
add comment on expression and statements in AllocatorAwareContainer
m-fila May 9, 2024
0f4975d
fix contextually convertible, add multipass guarantee
m-fila May 9, 2024
9ebbeea
add placeholders for LegacyOutputIterator
m-fila May 9, 2024
cd4ea94
add adaptors table, algorithms table
m-fila May 9, 2024
01b5427
add comment on algorithms and ranges
m-fila May 9, 2024
b5065fb
add iterator concepts table, move iterator concepts to separate test
m-fila May 9, 2024
d8dabe5
add tests for dereference assignment (and increment)
m-fila May 9, 2024
b3760e0
updated adaptors
m-fila May 9, 2024
1a63c21
Return type aliases in UserDataContainer begin and end methods
m-fila May 14, 2024
55e77e5
Don't use decltype in UserDataCollection type aliases
m-fila May 14, 2024
b6ab5d3
reorder sections, fix consistency and typos
m-fila May 21, 2024
04ea18a
rename check macro, check dereference assignment, duplicate tests for…
m-fila May 21, 2024
f79ac29
fix commented test, add some comments, use static_require_false inst…
m-fila May 21, 2024
4c1afbb
don't create separate target for tests
m-fila May 21, 2024
b844c09
add comments, reorganize test sections
m-fila May 22, 2024
e3356b7
fix pre-commit
m-fila May 22, 2024
04db87f
fix iterators are not swappable
m-fila May 24, 2024
4975d44
fix not needed requirement, fix typo
m-fila May 24, 2024
0f733e2
fix typo
m-fila May 25, 2024
3fdf08b
fix cv and ref qualifiers, add traits for * and ->
m-fila May 25, 2024
75c1414
fix trait consistency
m-fila May 25, 2024
107dacb
add mention the rest of legacy iterators
m-fila May 25, 2024
72109a1
add check iterator_category
m-fila May 25, 2024
e25de51
add clarification *mutable* in std iterators vs *mutable* in podio
m-fila May 25, 2024
a7248b0
fix typos
m-fila May 25, 2024
eb4a493
add checks for adaptors
m-fila May 25, 2024
b3d953d
add checks for move iterator adaptor
m-fila May 26, 2024
f86ac42
fix typos
m-fila May 26, 2024
5d7dd38
add collection vs container docs to index
m-fila Jun 3, 2024
7477890
add paragraph on internals
m-fila Jun 4, 2024
a55733c
clarify container value_type
m-fila Jun 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 183 additions & 0 deletions doc/collections_as_container.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
# PODIO Collection as a *Container*

Comparison of the PODIO `Collection`s with a C++ named requirement [*Container*](https://en.cppreference.com/w/cpp/named_req/Container).

The PODIO `Collection`s interface was designed to mimic the standard *Container* interface, in particular `std::vector`. Perfect compliance with the *Container* is not achieved as the `Collection`s are concerned with additional semantics such as mutable/immutable element access, associations and relations, and IO which that are not part of *Container*.

On the implementation level most of the differences with respect to the *Container* comes from the fact that in order to satisfy the additional semantics a `Collection` doesn't directly store [user layer objects](design.md#the-user-layer). Instead, [data layer objects](design.md#the-internal-data-layer) are stored and user layer objects are constructed and returned when needed. Similarly, the `Collection` iterators operate on the user layer objects but don't expose `Collection`'s storage directly to the users. Instead, they construct and return user layer objects when needed.
In other words, a `Collection` utilizes the user layer type as a reference type instead of using plain references (`&` or `&&`) to stored data layer types.

As a consequence some of the **standard algorithms may not work** with PODIO `Collection` iterators. See [standard algorithm documentation](#collection-and-standard-algorithms) below.

The following tables list the compliance of a PODIO generated collection with the *Container* named requirement, stating which member types, interfaces, or concepts are fulfilled and which are not. Additionally, there are some comments explaining missing parts or pointing out differences in behaviour.

m-fila marked this conversation as resolved.
Show resolved Hide resolved
### Container Types

| Name | Type | Requirements | Fulfilled by Collection? | Comment |
|------|------|--------------|--------------------------|---------|
| `value_type` | `T` | *[Erasable](https://en.cppreference.com/w/cpp/named_req/Erasable)* | ✔️ yes | Defined as an immutable user layer object type |
| `reference` | `T&` | | ❌ no | Not defined |
| `const_reference` | `const T&` | | ❌ no | Not defined |
| `iterator` | Iterator whose `value_type` is `T` | [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) convertible to `const_iterator` | ❌ no | Defined as podio `MutableCollectionIterator`. `iterator::value_type` not defined, not [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) ([see below](#legacyforwarditerator)), not convertible to `const_iterator`|
| `const_iterator` | Constant iterator whose `value_type` is `T` | [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) | ❌ no | Defined as podio `CollectionIterator`. `const_iterator::value_type` not defined, not [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) ([see below](#legacyforwarditerator))
| `difference_type`| Signed integer | Must be the same as `std::iterator_traits::difference_type` for `iterator` and `const_iterator` | ❌ no | `std::iterator_traits::difference_type` not defined |
| `size_type` | Unsigned integer | Large enough to represent all positive values of `difference_type` | ✔️ yes | |

### Container member functions and operators

| Expression | Return type | Semantics | Fulfilled by Collection? | Comment |
|------------|-------------|-----------|--------------------------|---------|
| `C()` | `C` | Creates an empty container | ✔️ yes | |
| `C(a)` | `C` | Creates a copy of `a` | ❌ no | Not defined, non-copyable by design |
| `C(rv)` | `C` | Moves `rv` | ✔️ yes | |
| `a = b` | `C&` | Destroys or copy-assigns all elements of `a` from elements of `b` | ❌ no | Not defined, non-copyable by design |
| `a = rv` | `C&` | Destroys or move-assigns all elements of `a` from elements of `rv` | ✔️ yes | |
| `a.~C()` | `void` | Destroys all elements of `a` and frees all memory| ✔️ yes | Invalidates all handles retrieved from this collection |
| `a.begin()` | `(const_)iterator` | Iterator to the first element of `a` | ✔️ yes | |
| `a.end()` | `(const_)iterator` | Iterator to one past the last element of `a` | ✔️ yes | |
| `a.cbegin()` | `const_iterator` | Same as `const_cast<const C&>(a).begin()` | ✔️ yes | |
| `a.cend()` | `const_iterator` | Same as `const_cast<const C&>(a).end()`| ✔️ yes | |
| `a == b` | Convertible to `bool` | Same as `std::equal(a.begin(), a.end(), b.begin(), b.end())`| ❌ no | Not defined |
| `a != b` | Convertible to `bool` | Same as `!(a == b)` | ❌ no | Not defined |
| `a.swap(b)` | `void` | Exchanges the values of `a` and `b` | ❌ no | Not defined |
| `swap(a,b)` | `void` | Same as `a.swap(b)` | ❌ no | `a.swap(b)` not defined |
| `a.size()` | `size_type` | Same as `std::distance(a.begin(), a.end())` | ✔️ yes | |
| `a.max_size()` | `size_type` | `b.size()` where b is the largest possible container | ✔️ yes | |
| `a.empty()` | Convertible to `bool` | Same as `a.begin() == a.end()` | ✔️ yes | |

## Collection as an *AllocatorAwareContainer*

The C++ standard specifies [AllocatorAwareContainer](https://en.cppreference.com/w/cpp/named_req/AllocatorAwareContainer) for containers that can use other allocators beside the default allocator.

PODIO collections don't provide a customization point for allocators and use only the default allocator. Therefore they are not *AllocatorAwareContainers*.

### AllocatorAwareContainer types

| Name | Requirements | Fulfilled by Collection? | Comment |
|------|--------------|--------------------------|---------|
| `allocator_type` | `allocator_type::value_type` same as `value_type` | ❌ no | `allocator_type` not defined |

### *AllocatorAwareContainer* expression and statements

The PODIO Collections currently are not checked against expression and statements requirements for *AllocatorAwareContainer*.

## Collection iterators as an *Iterator*

The C++ specifies a set of named requirements for iterators. Starting with C++20 the standard specifies also iterator concepts. The requirements imposed by the concepts and named requirements are similar but not identical.

In the following tables a convention from `Collection` is used: `iterator` stands for PODIO `MutableCollectionIterator` and `const_iterator` stands for PODIO `CollectionIterator`.
### Iterator summary

| Named requirement | `iterator` | `const_iterator` |
|-------------------|-----------------------|-----------------------------|
| [LegacyIterator](https://en.cppreference.com/w/cpp/named_req/Iterator) | ❌ no ([see below](#legacyiterator)) | ❌ no ([see below](#legacyiterator)) |
| [LegacyInputIterator](https://en.cppreference.com/w/cpp/named_req/InputIterator) | ❌ no ([see below](#legacyinputiterator)) | ❌ no ([see below](#legacyinputiterator)) |
| [LegacyForwardIterator](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) | ❌ no ([see below](#legacyforwarditerator)) | ❌ no ([see below](#legacyforwarditerator)) |
| [LegacyOutputIterator](https://en.cppreference.com/w/cpp/named_req/OutputIterator) | ❌ no ([see below](#legacyoutputiterator)) | ❌ no ([see below](#legacyoutputiterator)) |
| [LegacyBidirectionalIterator](https://en.cppreference.com/w/cpp/named_req/BidirectionalIterator) | ❌ no | ❌ no |
| [LegacyRandomAccessIterator](https://en.cppreference.com/w/cpp/named_req/RandomAccessIterator) | ❌ no | ❌ no |
| [LegacyContiguousIterator](https://en.cppreference.com/w/cpp/named_req/ContiguousIterator) | ❌ no | ❌ no |

| Concept | `iterator` | `const_iterator` |
|---------|------------------------|------------------------------|
| `std::indirectly_readable` | ❌ no | ❌ no |
| `std::indirectly_writable` | ❌ no | ❌ no |
| `std::weakly_incrementable` | ❌ no | ❌ no |
| `std::incrementable` | ❌ no | ❌ no |
| `std::input_or_output_iterator` | ❌ no | ❌ no |
| `std::input_iterator` | ❌ no | ❌ no |
| `std::output_iterator` | ❌ no | ❌ no |
| `std::forward_iterator` | ❌ no | ❌ no |
| `std::bidirectional_iterator` | ❌ no | ❌ no |
| `std::random_access_iterator` | ❌ no | ❌ no |
| `std::contiguous_iterator` | ❌ no | ❌ no |

### LegacyIterator

| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment |
|-------------|-------------------------------------------|---------|
| [*CopyConstructible*](https://en.cppreference.com/w/cpp/named_req/CopyConstructible) | ❌ no / ❌ no | Move constructor and copy constructor not defined |
| [*CopyAssignable*](https://en.cppreference.com/w/cpp/named_req/CopyAssignable) | ❌ no / ❌ no | Move assignment and copy assignment not defined |
| [*Destructible*](https://en.cppreference.com/w/cpp/named_req/Destructible) | ✔️ yes / ✔️ yes | |
| [*Swappable*](https://en.cppreference.com/w/cpp/named_req/Swappable) | ❌ no / ❌ no | |
| `std::iterator_traits::value_type` (Until C++20 ) | ❌ no / ❌ no | Not defined |
| `std::iterator_traits::difference_type` | ❌ no / ❌ no | Not defined |
| `std::iterator_traits::reference` | ❌ no / ❌ no | Not defined |
| `std::iterator_traits::pointer` | ❌ no / ❌ no | Not defined |
| `std::iterator_traits::iterator_category` | ❌ no / ❌ no | Not defined |

| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment |
|------------|-------------|-----------|-------------------------------------------|---------|
| `*r` | Unspecified | | ✔️ yes / ✔️ yes | |
| `++r` | `It&` | | ✔️ yes / ✔️ yes | |

### LegacyInputIterator

| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment |
|-------------|-------------------------------------------|---------|
| [*LegacyIterator*](https://en.cppreference.com/w/cpp/named_req/Iterator) | ❌ no / ❌ no | [See above](#legacyiterator) |
| [*EqualityComparable*](https://en.cppreference.com/w/cpp/named_req/EqualityComparable) | ✔️ yes / ✔️ yes | |

| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment |
|------------|-------------|-----------|-------------------------------------------|---------|
| `i != j` | Contextually convertible to `bool` | Same as `!(i==j)` | ✔️ yes / ✔️ yes | |
| `*i` | `reference`, convertible to `value_type` | | ❌ no / ❌ no | `reference` and `value_type` not defined |
| `i->m` | | Same as `(*i).m` | ✔️ yes / ✔️ yes | |
| `++r` | `It&` | | ✔️ yes / ✔️ yes | |
| `(void)r++` | | Same as `(void)++r` | ❌ no / ❌ no | Post-increment not defined |
| `*r++` | Convertible to `value_type` | Same as `value_type x = *r; ++r; return x;` | ❌ no / ❌ no | Post-increment and `value_type` not defined |

### LegacyForwardIterator

In addition to the *LegacyForwardIterator* the C++ standard specifies also the *mutable LegacyForwardIterator*, which is both *LegacyForwardIterator* and *LegacyOutputIterator*. The term **mutable** used in this context doesn't imply mutability in the sense used in the PODIO.


| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment |
|-------------|-------------------------------------------|---------|
| [*LegacyInputIterator*](https://en.cppreference.com/w/cpp/named_req/InputIterator) | ❌ no / ❌ no | [See above](#legacyinputiterator)|
| [*DefaultConstructible*](https://en.cppreference.com/w/cpp/named_req/DefaultConstructible) | ❌ no / ❌ no | Value initialization not defined |
| If *mutable* iterator then `reference` same as `value_type&` or `value_type&&`, otherwise same as `const value_type&` or `const value_type&&` | ❌ no / ❌ no | `reference` and `value_type` not defined |
| [Multipass guarantee](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) | ❌ no / ❌ no | Copy constructor not defined |
| [Singular iterators](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) | ❌ no / ❌ no | Value initialization not defined |

| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment |
|------------|-------------|-----------|-------------------------------------------|---------|
| `i++` | `It` | Same as `It ip = i; ++i; return ip;` | ❌ no / ❌ no | Post-increment not defined |
| `*i++` | `reference` | | ❌ no / ❌ no | Post-increment and `reference` not defined |

### LegacyOutputIterator

| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment |
|-------------|-------------------------------------------|---------|
| [*LegacyIterator*](https://en.cppreference.com/w/cpp/named_req/Iterator) | ❌ no / ❌ no | [See above](#legacyiterator) |
| Is pointer type or class type | ✔️ yes / ✔️ yes | |

| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment |
|------------|-------------|-----------|-------------------------------------------|---------|
| `*r = o` | | | ❗ attention / ❗ attention | Defined but an assignment doesn't modify objects inside collection |
| `++r` | `It&` | | ✔️ yes / ✔️ yes | |
| `r++` | Convertible to `const It&` | Same as `It temp = r; ++r; return temp;` | ❌ no / ❌ no | Post-increment not defined |
| `*r++ = o` | | Same as `*r = o; ++r;`| ❌ no / ❌ no | Post-increment not defined |

## Collection iterators and standard iterator adaptors

| Adaptor | Compatible with Collection? | Comment |
|---------|-----------------------------|---------|
| `std::reverse_iterator` | ❌ no | `iterator` and `const_iterator` not *LegacyBidirectionalIterator* or `std::bidirectional_iterator` |
| `std::back_insert_iterator` | ❗ attention | Compatible only with SubsetCollections, otherwise throws `std::invalid_argument` |
| `std::front_insert_iterator` | ❌ no | `push_front` not defined |
| `std::insert_iterator` | ❌ no | `insert` not defined |
| `std::const_iterator` | ❌ no | `iterator` and `const_iterator` not *LegacyInputIterator* or `std::input_iterator` |
| `std::move_iterator` | ❌ no | `iterator` and `const_iterator` not *LegacyInputIterator* or `std::input_iterator` |
| `std::counted_iterator` | ❌ no | `iterator` and `const_iterator` not `std::input_or_output_iterator` |


## Collection and standard algorithms

Most of the standard algorithms require the iterators to be at least *InputIterator*. The iterators of PODIO collection don't fulfil this requirement, therefore they are not compatible with standard algorithms according to the specification.

In practice, some algorithms may still compile with the collections depending on the implementation of a given algorithm. In general, the standard **algorithms mutating a collection will give wrong results**, while the standard algorithms not mutating a collection in principle should give correct results if they compile.

## Standard range algorithms

The standard range algorithm use constrains to operate at least on `std::input_iterator`s and `std::ranges::input_range`s. The iterators of PODIO collection don't model these concepts, therefore can't be used with standard range algorithms. The range algorithms won't compile with PODIO `Collection` iterators.
1 change: 1 addition & 0 deletions doc/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@ Welcome to PODIO's documentation!
advanced_topics.md
templates.md
python.md
collections_as_container.md
cpp_api/api
py_api/modules
3 changes: 3 additions & 0 deletions include/podio/CollectionBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ class CollectionBase {
/// number of elements in the collection
virtual size_t size() const = 0;

/// maximal number of elements in the collection
virtual std::size_t max_size() const = 0;

/// Is the collection empty
virtual bool empty() const = 0;

Expand Down
25 changes: 21 additions & 4 deletions include/podio/UserDataCollection.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ class UserDataCollection : public CollectionBase {
VectorMembersInfo m_vecmem_info{};

public:
using value_type = typename std::vector<BasicType>::value_type;
using const_iterator = typename std::vector<BasicType>::const_iterator;
using iterator = typename std::vector<BasicType>::iterator;
using difference_type = typename std::vector<BasicType>::difference_type;
using size_type = typename std::vector<BasicType>::size_type;

UserDataCollection() = default;
/// Constructor from an existing vector (which will be moved from!)
UserDataCollection(std::vector<BasicType>&& vec) : _vec(std::move(vec)) {
Expand Down Expand Up @@ -133,6 +139,11 @@ class UserDataCollection : public CollectionBase {
return _vec.size();
}

/// maximal number of elements in the collection
size_t max_size() const override {
return _vec.max_size();
}

/// Is the collection empty
bool empty() const override {
return _vec.empty();
Expand Down Expand Up @@ -194,18 +205,24 @@ class UserDataCollection : public CollectionBase {

// ----- some wrappers for std::vector and access to the complete std::vector (if really needed)

typename std::vector<BasicType>::iterator begin() {
iterator begin() {
return _vec.begin();
}
typename std::vector<BasicType>::iterator end() {
iterator end() {
return _vec.end();
}
typename std::vector<BasicType>::const_iterator begin() const {
const_iterator begin() const {
return _vec.begin();
}
typename std::vector<BasicType>::const_iterator end() const {
const_iterator end() const {
return _vec.end();
}
const_iterator cbegin() const {
return _vec.cbegin();
}
const_iterator cend() const {
return _vec.cend();
}

typename std::vector<BasicType>::reference operator[](size_t idx) {
return _vec[idx];
Expand Down
4 changes: 4 additions & 0 deletions python/templates/Collection.cc.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ std::size_t {{ collection_type }}::size() const {
return m_storage.entries.size();
}

std::size_t {{ collection_type }}::max_size() const {
return m_storage.entries.max_size();
}

bool {{ collection_type }}::empty() const {
return m_storage.entries.empty();
}
Expand Down
11 changes: 11 additions & 0 deletions python/templates/Collection.h.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ public:
using value_type = {{ class.bare_type }};
using const_iterator = {{ class.bare_type }}CollectionIterator;
using iterator = {{ class.bare_type }}MutableCollectionIterator;
using difference_type = ptrdiff_t;
using size_type = size_t;

{{ class.bare_type }}Collection();
{{ class.bare_type }}Collection({{ class.bare_type }}CollectionData&& data, bool isSubsetColl);
Expand Down Expand Up @@ -86,6 +88,9 @@ public:
/// number of elements in the collection
std::size_t size() const final;

/// maximal number of elements in the collection
std::size_t max_size() const final;

/// Is the collection empty
bool empty() const final;

Expand Down Expand Up @@ -153,12 +158,18 @@ public:
const_iterator begin() const {
return const_iterator(0, &m_storage.entries);
}
const_iterator cbegin() const {
return begin();
}
iterator end() {
return iterator(m_storage.entries.size(), &m_storage.entries);
}
const_iterator end() const {
return const_iterator(m_storage.entries.size(), &m_storage.entries);
}
const_iterator cend() const {
return end();
}

{% for member in Members %}
std::vector<{{ member.full_type }}> {{ member.name }}(const size_t nElem = 0) const;
Expand Down
2 changes: 1 addition & 1 deletion tests/unittests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ if(NOT Catch2_FOUND)
endif()

find_package(Threads REQUIRED)
add_executable(unittest_podio unittest.cpp frame.cpp buffer_factory.cpp interface_types.cpp)
add_executable(unittest_podio unittest.cpp frame.cpp buffer_factory.cpp interface_types.cpp std_interoperability.cpp)
target_link_libraries(unittest_podio PUBLIC TestDataModel PRIVATE Catch2::Catch2WithMain Threads::Threads podio::podioRootIO)
if (ENABLE_SIO)
target_link_libraries(unittest_podio PRIVATE podio::podioSioIO)
Expand Down
Loading