root-project / root

The official repository for ROOT: analyzing, storing and visualizing big data, scientifically
https://root.cern
Other
2.64k stars 1.26k forks source link

[Tree] Bogus data silently read when trying to access an indexed friend TTree with an invalid index #7713

Open eguiraud opened 3 years ago

eguiraud commented 3 years ago
eguiraud commented 2 years ago

Adding the 6.26 milestone after discussion with Axel

pcanal commented 1 year ago

We have a related/similar issue with a TChain containing trees with missing branches:

f = new TFile("f1.root","RECREATE");
t1 = new TTree("t1","t1a")
int i = 55;
t1->Branch("a",&i);
t1->Branch("b",&i);
t1->Fill();
f->Write();
f = new TFile("f2.root","RECREATE");
t1 = new TTree("t1","t1b")
int i = -55;
t1->Branch("a",&i);
t1->Fill();
f->Write();
.q
c = new TChain("t1");
c->Scan("a")
c->Add("f1.root");
c->Add("f2.root");
c->Scan("a")
c->Scan("b")
c->Scan("")

prints:

************************
*    Row   *         a *
************************
*        0 *        55 *
*        1 *       -55 *
************************
(long long) 2
root [5] c->Scan("b")
************************
*    Row   *         b *
************************
*        0 *        55 *
*        1 *         0 *
************************
(long long) 2
root [6] c->Scan("")
************************************
*    Row   *       a.a *       b.b *
************************************
*        0 *        55 *        55 *
*        1 *       -55 *         0 *
************************************
Axel-Naumann commented 11 months ago

Do I understand correctly that this is in fact fixed? by #12260 ? Can this be closed? If not - do we need to delay v6.30/00 because of it?

eguiraud commented 11 months ago

Hi, this is still an issue. TChain and TTree silently return the last valid value for an indexed friend tree if the main one asks for a non-existing index.

12260 was a specific bug in RDF's multi-threading + indexed friend trees that users encountered and that would have been made immediately visible if this issue was fixed.

Axel-Naumann commented 11 months ago

So do we need to delay 6.30/00 because of this, @pcanal ? I guess not - my suspicion is that this exists since the beginning of time?

eguiraud commented 11 months ago

This definitely exists since the beginning of time, and I completely understand the situation with the release. In fact, the same thing happened with 6.26 and then 6.28.

Just allow me to say that just because it's an old bug it does not make it less evil :smile:

Axel-Naumann commented 11 months ago

It's critical 🚨

guitargeek commented 4 months ago

Should we maybe remove the "critical" label?

TomasDado commented 2 months ago

Hi, this issue is causing us problems running differential cross-section measurements using RDataFrame. It is very common that you have events passing the detector level selection but not the truth level selection (often separate trees that you need to match). And I am not aware of any workaround directly from RDataFrame. We discussed a bit with @vepadulano and it would be ideal to provide a function that would allow users to check if a given friend event exists - this would allow the option for the users to filter on this and skip the problematic events.