Closed tdietel closed 6 months ago
guitargeek
commented
1 year ago Hi! I think @Axel-Naumann is able to tell authoritatively if this is within the scope of the TTreeReaderArray or not.
I think it's not. You'll have to copy the elements to an STL collection and then sort the new collection.
Axel-Naumann
commented
1 year ago Thanks for the report!
I would love to mark the TTreeReaderArray iterators const but that will break the code of people who don't mark their member functions const... So indeed while sorting on the iterators (and manipulation of the buffer TTreeReaderArray operates on) is not supported I don't know how to signal that.
Any ideas?
dpiparo
commented
1 year ago I thought about this and unfortunately I did not have any good idea yet. However, if the problem to be solved is sorting the entries in TTreeReaderArrays without additional copies, a solution could be working on indices (apologies if this is not fitting this particular use case in ALICE). For example
template <class MyCollection>
std::vector<size_t> ArgStableSort(const MyCollection &mycoll)
{
std::vector<size_t> sortedIndices(mycoll.size());
std::iota(sortedIndices.begin(),sortedIndices.end(),0);
std::stable_sort( sortedIndices.begin(), sortedIndices.end(), [&](int i,int j){return mycoll[i]<mycoll[j];});
return sortedIndices;
}
// [....]
std::vector<char> myvec{'c', 'e', 'd', 'a', 'b', 'f'};
const auto myvec_sidx = ArgStableSort(myvec);
for (size_t idx : myvec_sidx) {
std::cout << myvec[idx] << std::endl;
}This is an approach used in many libraries, including ROOT's VecOps
guitargeek
commented
6 months ago That's a nice demo, @dpiparo, and how it's commonly done. Or you can just copy the collection, if it's not too expensive.
As for the actual issue: it should be clear that reader views should not be sortable. We can't make this explicitly clear with const because ROOT and it's users are not very consistent with that keyword, as @Axel-Naumann said. So there is no solution here - ROOT and C++ just allow you to shoot yourself in the foot :slightly_smiling_face:
Check duplicate issues.
Description
I tried to run std::stable_sort on a TTreeReaderArrray. The sorting works fine on macOS, but a colleague tried to run the same code on Ubuntu and reported a crash. I then managed to reproduce the crash on CentOS 7.
I investigated the error, and it seems that std::stable_sort assumes that it can decrement and invalid pointer to get a pointer to the last array of an element. However, TTreeReaderArray invalidates the iterator by setting fArray=0 when fIndex is incremented to fIndex>=fSize. This means that the information about the associated array is lost, and the subsequent decrement and dereference will fail. I would assume this could be fixed by marking an invalid iterator with a special value for fIndex (instead of setting fArray=0).
So my question would be if TTreeReaderArray is supposed to be sortable, or if I am trying something that's outside of the scope of the implementation?
I know my reproducer requires ALICE O2.
Reproducer
I encountered the bug with a tree that holds classes from the ALICE O2 framework. I wrote a minimal macro
sortbug.Cthat fails when run in compiled mode in an O2 root environment. Data files are available from https://cernbox.cern.ch/s/ypjoEC6awvYpjwR. The command is rootsortbug.C+. The macro content is:The code creates the following stack trace:
I assume the bug can be triggered with other data sets in plain ROOT. If needed, I could try to strip things down, but I don't know if this is the most efficient use of my time.
ROOT version
Installation method
Built with alibuild
Operating system
CentOS 7, Ubuntu
Additional context
No response