From ccfb7a0a156b3ebc02a873f657495dd81c96de29 Mon Sep 17 00:00:00 2001 From: Grigori Rybkine Date: Thu, 7 May 2026 12:44:45 +0200 Subject: [PATCH 1/3] [ntuple][ATLAS experiment] Make RNTupleDescriptor `RIterator`s usable with STL algorithms tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx: ensure that the following iterators RNTupleDescriptor::RFieldDescriptorIterable::RIterator, RNTupleDescriptor::RClusterDescriptorIterable::RIterator, RNTupleDescriptor::RColumnDescriptorIterable::RIterator, RNTupleDescriptor::RColumnRangeIterable::RIterator, RNTupleDescriptor::RClusterGroupDescriptorIterable::RIterator DefaultConstructible, RNTupleDescriptor::RExtraTypeInfoDescriptorIterable::RIterator, Experimental::RNTupleAttrSetDescriptorIterable::RIterator meet the requirements of Cpp17ForwardIterator or LegacyForwardIterator in the terminology of en.cppreference.com site --- tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx | 81 +++++++++++++--------- 1 file changed, 47 insertions(+), 34 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx b/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx index 9d4426af547af..51c9aa8ce4164 100644 --- a/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx @@ -544,7 +544,7 @@ public: using pointer = const RColumnRange *; using reference = const RColumnRange &; - RIterator(Iter_t iter) : fIter(iter) {} + explicit RIterator(Iter_t iter = Iter_t{}) : fIter(iter) {} iterator &operator++() /* prefix */ { ++fIter; @@ -953,11 +953,9 @@ private: public: class RIterator final { private: - /// The enclosing range's RNTuple. - const RNTupleDescriptor &fNTuple; - /// The enclosing range's descriptor id list. - const std::vector &fColumns; - std::size_t fIndex = 0; + /// The enclosing RColumnDescriptorIterable. + const RColumnDescriptorIterable *fIterable; + std::size_t fIndex; public: using iterator_category = std::forward_iterator_tag; @@ -967,8 +965,8 @@ public: using pointer = const RColumnDescriptor *; using reference = const RColumnDescriptor &; - RIterator(const RNTupleDescriptor &ntuple, const std::vector &columns, std::size_t index) - : fNTuple(ntuple), fColumns(columns), fIndex(index) + explicit RIterator(const RColumnDescriptorIterable *iterable = nullptr, std::size_t index = 0) + : fIterable(iterable), fIndex(index) { } iterator &operator++() /* prefix */ @@ -982,17 +980,26 @@ public: operator++(); return old; } - reference operator*() const { return fNTuple.GetColumnDescriptor(fColumns.at(fIndex)); } - pointer operator->() const { return &fNTuple.GetColumnDescriptor(fColumns.at(fIndex)); } - bool operator!=(const iterator &rh) const { return fIndex != rh.fIndex; } - bool operator==(const iterator &rh) const { return fIndex == rh.fIndex; } + reference operator*() const + { + if (fIterable) + return fIterable->fNTuple.GetColumnDescriptor(fIterable->fColumns.at(fIndex)); + throw RException(R__FAIL("dereference of RNTupleDescriptor::RColumnDescriptorIterable::RIterator" + " constructed without RNTupleDescriptor::RColumnDescriptorIterable")); + } + pointer operator->() const + { + return fIterable ? &fIterable->fNTuple.GetColumnDescriptor(fIterable->fColumns.at(fIndex)) : nullptr; + } + bool operator!=(const iterator &rh) const { return fIndex != rh.fIndex || fIterable != rh.fIterable; } + bool operator==(const iterator &rh) const { return fIndex == rh.fIndex && fIterable == rh.fIterable; } }; RColumnDescriptorIterable(const RNTupleDescriptor &ntuple, const RFieldDescriptor &fieldDesc); RColumnDescriptorIterable(const RNTupleDescriptor &ntuple); - RIterator begin() { return RIterator(fNTuple, fColumns, 0); } - RIterator end() { return RIterator(fNTuple, fColumns, fColumns.size()); } + RIterator begin() { return RIterator(this, 0); } + RIterator end() { return RIterator(this, fColumns.size()); } size_t size() { return fColumns.size(); } }; @@ -1014,11 +1021,9 @@ private: public: class RIterator final { private: - /// The enclosing range's RNTuple. - const RNTupleDescriptor &fNTuple; - /// The enclosing range's descriptor id list. - const std::vector &fFieldChildren; - std::size_t fIndex = 0; + /// The enclosing RFieldDescriptorIterable. + const RFieldDescriptorIterable *fIterable; + std::size_t fIndex; public: using iterator_category = std::forward_iterator_tag; @@ -1028,9 +1033,8 @@ public: using pointer = const RFieldDescriptor *; using reference = const RFieldDescriptor &; - RIterator(const RNTupleDescriptor &ntuple, const std::vector &fieldChildren, - std::size_t index) - : fNTuple(ntuple), fFieldChildren(fieldChildren), fIndex(index) + explicit RIterator(const RFieldDescriptorIterable *iterable = nullptr, std::size_t index = 0) + : fIterable(iterable), fIndex(index) { } iterator &operator++() /* prefix */ @@ -1044,10 +1048,19 @@ public: operator++(); return old; } - reference operator*() const { return fNTuple.GetFieldDescriptor(fFieldChildren.at(fIndex)); } - pointer operator->() const { return &fNTuple.GetFieldDescriptor(fFieldChildren.at(fIndex)); } - bool operator!=(const iterator &rh) const { return fIndex != rh.fIndex; } - bool operator==(const iterator &rh) const { return fIndex == rh.fIndex; } + reference operator*() const + { + if (fIterable) + return fIterable->fNTuple.GetFieldDescriptor(fIterable->fFieldChildren.at(fIndex)); + throw RException(R__FAIL("dereference of RNTupleDescriptor::RFieldDescriptorIterable::RIterator" + " constructed without RNTupleDescriptor::RFieldDescriptorIterable")); + } + pointer operator->() const + { + return fIterable ? &fIterable->fNTuple.GetFieldDescriptor(fIterable->fFieldChildren.at(fIndex)) : nullptr; + } + bool operator!=(const iterator &rh) const { return fIndex != rh.fIndex || fIterable != rh.fIterable; } + bool operator==(const iterator &rh) const { return fIndex == rh.fIndex && fIterable == rh.fIterable; } }; RFieldDescriptorIterable(const RNTupleDescriptor &ntuple, const RFieldDescriptor &field) : fNTuple(ntuple), fFieldChildren(field.GetLinkIds()) @@ -1060,8 +1073,8 @@ public: { std::sort(fFieldChildren.begin(), fFieldChildren.end(), comparator); } - RIterator begin() { return RIterator(fNTuple, fFieldChildren, 0); } - RIterator end() { return RIterator(fNTuple, fFieldChildren, fFieldChildren.size()); } + RIterator begin() { return RIterator(this, 0); } + RIterator end() { return RIterator(this, fFieldChildren.size()); } }; // clang-format off @@ -1093,7 +1106,7 @@ public: using pointer = const RClusterGroupDescriptor *; using reference = const RClusterGroupDescriptor &; - RIterator(Iter_t iter) : fIter(iter) {} + explicit RIterator(Iter_t iter = Iter_t{}) : fIter(iter) {} iterator &operator++() /* prefix */ { ++fIter; @@ -1147,7 +1160,7 @@ public: using pointer = const RClusterDescriptor *; using reference = const RClusterDescriptor &; - RIterator(Iter_t iter) : fIter(iter) {} + explicit RIterator(Iter_t iter = Iter_t{}) : fIter(iter) {} iterator &operator++() /* prefix */ { ++fIter; @@ -1197,7 +1210,7 @@ public: using pointer = const RExtraTypeInfoDescriptor *; using reference = const RExtraTypeInfoDescriptor &; - RIterator(Iter_t iter) : fIter(iter) {} + explicit RIterator(Iter_t iter = Iter_t{}) : fIter(iter) {} iterator &operator++() /* prefix */ { ++fIter; @@ -1210,7 +1223,7 @@ public: return old; } reference operator*() const { return *fIter; } - pointer operator->() const { return &*fIter; } + pointer operator->() const { return fIter.operator->(); } bool operator!=(const iterator &rh) const { return fIter != rh.fIter; } bool operator==(const iterator &rh) const { return fIter == rh.fIter; } }; @@ -1249,7 +1262,7 @@ public: using pointer = const value_type *; using reference = const value_type &; - RIterator(Iter_t iter) : fIter(iter) {} + explicit RIterator(Iter_t iter = Iter_t{}) : fIter(iter) {} iterator &operator++() /* prefix */ { ++fIter; @@ -1262,7 +1275,7 @@ public: return old; } reference operator*() const { return *fIter; } - pointer operator->() const { return &*fIter; } + pointer operator->() const { return fIter.operator->(); } bool operator!=(const iterator &rh) const { return fIter != rh.fIter; } bool operator==(const iterator &rh) const { return fIter == rh.fIter; } }; From ef221338d0d534c4f9b538612ceefa02dd61ef64 Mon Sep 17 00:00:00 2001 From: Grigori Rybkine Date: Thu, 14 May 2026 21:37:48 +0200 Subject: [PATCH 2/3] [ntuple][ATLAS experiment] Simplify default constructor of RNTupleDescriptor `RIterator`s tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx: make the iterator default constructor explicitly-defaulted --- tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx b/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx index 51c9aa8ce4164..388835053d482 100644 --- a/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx @@ -544,7 +544,8 @@ public: using pointer = const RColumnRange *; using reference = const RColumnRange &; - explicit RIterator(Iter_t iter = Iter_t{}) : fIter(iter) {} + RIterator() = default; + explicit RIterator(Iter_t iter) : fIter(iter) {} iterator &operator++() /* prefix */ { ++fIter; @@ -960,7 +961,7 @@ public: public: using iterator_category = std::forward_iterator_tag; using iterator = RIterator; - using value_type = RFieldDescriptor; + using value_type = RColumnDescriptor; using difference_type = std::ptrdiff_t; using pointer = const RColumnDescriptor *; using reference = const RColumnDescriptor &; @@ -1106,7 +1107,8 @@ public: using pointer = const RClusterGroupDescriptor *; using reference = const RClusterGroupDescriptor &; - explicit RIterator(Iter_t iter = Iter_t{}) : fIter(iter) {} + RIterator() = default; + explicit RIterator(Iter_t iter) : fIter(iter) {} iterator &operator++() /* prefix */ { ++fIter; @@ -1160,7 +1162,8 @@ public: using pointer = const RClusterDescriptor *; using reference = const RClusterDescriptor &; - explicit RIterator(Iter_t iter = Iter_t{}) : fIter(iter) {} + RIterator() = default; + explicit RIterator(Iter_t iter) : fIter(iter) {} iterator &operator++() /* prefix */ { ++fIter; @@ -1210,7 +1213,8 @@ public: using pointer = const RExtraTypeInfoDescriptor *; using reference = const RExtraTypeInfoDescriptor &; - explicit RIterator(Iter_t iter = Iter_t{}) : fIter(iter) {} + RIterator() = default; + explicit RIterator(Iter_t iter) : fIter(iter) {} iterator &operator++() /* prefix */ { ++fIter; @@ -1262,7 +1266,8 @@ public: using pointer = const value_type *; using reference = const value_type &; - explicit RIterator(Iter_t iter = Iter_t{}) : fIter(iter) {} + RIterator() = default; + explicit RIterator(Iter_t iter) : fIter(iter) {} iterator &operator++() /* prefix */ { ++fIter; From a3400b503536e8d284fd9fb727af7d3162f017df Mon Sep 17 00:00:00 2001 From: Grigori Rybkine Date: Thu, 14 May 2026 23:09:51 +0200 Subject: [PATCH 3/3] [ntuple][ATLAS experiment] Update TestFromTTree method of RDFSnapshotRNTupleFromTTreeTest class tree/dataframe/test/dataframe_snapshot_ntuple.cxx: extend the field iterable lifetime while its iterator is in use by an algorithm --- tree/dataframe/test/dataframe_snapshot_ntuple.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tree/dataframe/test/dataframe_snapshot_ntuple.cxx b/tree/dataframe/test/dataframe_snapshot_ntuple.cxx index 9cfeb1298f138..d4e92e393a823 100644 --- a/tree/dataframe/test/dataframe_snapshot_ntuple.cxx +++ b/tree/dataframe/test/dataframe_snapshot_ntuple.cxx @@ -684,7 +684,8 @@ class RDFSnapshotRNTupleFromTTreeTest : public ::testing::Test { auto &descriptor = reader->GetDescriptor(); - int nTopLevelFields = std::distance(descriptor.GetTopLevelFields().begin(), descriptor.GetTopLevelFields().end()); + auto topLevelFieldsIterable = descriptor.GetTopLevelFields(); + int nTopLevelFields = std::distance(topLevelFieldsIterable.begin(), topLevelFieldsIterable.end()); EXPECT_EQ(9, nTopLevelFields); EXPECT_EQ("std::int32_t", descriptor.GetFieldDescriptor(descriptor.FindFieldId("x")).GetTypeName()); EXPECT_EQ("float", descriptor.GetFieldDescriptor(descriptor.FindFieldId("pt")).GetTypeName());