Repository navigation
Fix handling of projected fields in RNTupleProcessor chains - #23744
Conversation
49072fa to
62ad8f9
Compare
Test Results 24 files 24 suites 4d 0h 8m 26s ⏱️ For more details on these failures, see this check. Results for commit 478b6c5. ♻️ This comment has been updated with latest results. |
jblomer
left a comment
jblomer
left a comment
There was a problem hiding this comment.
In principle looks good to me. Regarding the term "artificial" in the commit and PR messages: the term "artificial field" is already taken for fields that are added due to schema evolution and that have no on-disk information. Here, I think the correct term is "projected field" (even though this is not a persistified projection).
Before, the field name `std::vector<float> jets` would be set to `_0`. Now, we traverse up the fieldInfos to find the actual field name and set it accordingly.
Projected fields adopt their on-disk ID from other fields. This commit adds a way to bookkeep how the on-disk ID of artificial fields should be set when they get updated.
62ad8f9 to
478b6c5
Compare
enirolf
changed the title
I've updated the wording 👍 |
Currently, the RNTupleProcessorDS crashes with chains of RNTuples that have collections, because the (non-persisted) projected cardinality fields cannot be reconnected properly when the next RNTuple is opened.
This fix adds an additional bookkeeping mechanism for projected fields. It keeps track of the RNTupleProcessorEntry index of the field it should get the on-disk ID from and sets it accordingly upon connection.
Additionally, tests with more involved RNTupleProcessor combinations are added.
There is one outstanding FIXME also uncovered by these tests, which is the
#colnamealias for collection sizes. This doesn't work yet for fields belonging to auxiliary RNTuples. It will be addressed in a future PR.