Operation: SetBranchAddress

Wile_E_Coyote · October 4, 2012, 5:42pm

PLAN 1
(Enable “TTree::ESetBranchAddressStatus” enum in user’s interpreted and compiled code.)

Edit the file:
./tree/tree/inc/TTree.h
and move the “enum ESetBranchAddressStatus” definition from the “protected:” to the “public:” part of the “class TTree”

Edit the file:
./tree/tree/inc/LinkDef.h
and add:
#pragma link C++ enum TTree::ESetBranchAddressStatus;

Wile_E_Coyote · October 4, 2012, 6:37pm

PLAN 2
(Fix “-5 / Missing branch” return value from TChain::SetBranchAddress.)

diff -u TChain.cxx.orig TChain.cxx

[code]— TChain.cxx.orig 2012-09-28 20:19:28.000000000 +0200
+++ TChain.cxx 2012-10-04 20:26:01.609628254 +0200
@@ -2288,6 +2288,9 @@
}
}
branch->SetAddress(add);

```
 } else {
```

    Error("SetBranchAddress", "unknown branch -> %s", bname);

```
    return kMissingBranch;
 }
```
} else {
if (ptr) {
[/code]

pcanal · October 4, 2012, 7:21pm

Hi,

Can you attach a patch file containing all the changes?

Thanks,
Philippe.

Wile_E_Coyote · October 4, 2012, 7:30pm

PLAN 3
(Fix “-1 and -2 / pointer type given does not match the type expected by the branch” return values from TChain::SetBranchAddress.)

For the moment I wasn’t able to fix it.
I believe that the problem is the following:

When the Int_t TChain::SetBranchAddress(const char bname, void add, TBranch** ptr) gets called, in line 2278 it executes res = CheckBranchAddressType(branch, TClass::GetClass(element->GetBaddressClassName()), (EDataType) element->GetBaddressType(), element->GetBaddressIsPtr()); Unfortunately, both “element” methods, “GetBaddressClassName” and “GetBaddressType”, return simply 0 (no matter what one tries). So, the “CheckBranchAddressType” is not able to check anything.
Note: the “element” can either be created directly in the TChain::SetBranchAddress" method itself, or it may have been created earlier by, for example, a “TChain::SetBranchStatus” call.

The difference with respect to the “TTree::SetBranchAddress” is that the “TTree.h” implements it as a templated method (which then internally knows the “classtype” and so on).

Templated “TChain::SetBranchAddress” methods are completely missing in “TChain.h”.
And so, I believe that the solution would be to add such methods (in the “TChain.h” file for the compiled code) plus one would need to add an appropriate trick for CINT (a special “manual” method as the one existing for the case of TTree for interpreted code?).

Wile_E_Coyote · October 4, 2012, 7:40pm

svn diff [code]Index: tree/tree/src/TChain.cxx

— tree/tree/src/TChain.cxx (revision 46321)
+++ tree/tree/src/TChain.cxx (working copy)
@@ -2288,6 +2288,9 @@
}
}
branch->SetAddress(add);

```
 } else {
```

    Error("SetBranchAddress", "unknown branch -> %s", bname);

```
    return kMissingBranch;
 }
```
} else {
if (ptr) {
Index: tree/tree/inc/LinkDef.h
===================================================================
— tree/tree/inc/LinkDef.h (revision 46321)
+++ tree/tree/inc/LinkDef.h (working copy)
@@ -67,6 +67,7 @@
#pragma link C++ class TBranchSTL+;
#pragma link C++ class TIndArray+;

+#pragma link C++ enum TTree::ESetBranchAddressStatus;

#pragma link C++ function operator+(const TCut&, const char);
#pragma link C++ function operator+(const char, const TCut&);
Index: tree/tree/inc/TTree.h

— tree/tree/inc/TTree.h (revision 46321)
+++ tree/tree/inc/TTree.h (working copy)
@@ -203,6 +203,8 @@
kSetBranchStatus = BIT(12)
};

+public:

// SetBranchAddress return values
enum ESetBranchAddressStatus {
kMissingBranch = -5,
kInternalError = -4,
@@ -217,7 +219,6 @@
kNoCheck = 5
};

-public:
// TTree status bits
enum {
kForceRead = BIT(11),[/code]

pcanal · October 4, 2012, 7:44pm

Hi,

I would prefer the result of svn diff -u …

Thanks,
Philippe.

Wile_E_Coyote · October 4, 2012, 7:55pm

post modified.

pcanal · October 4, 2012, 9:04pm

Thanks,
Now in the svn repository.

Philippe.

pcanal · October 4, 2012, 9:08pm

[quote]The difference with respect to the “TTree::SetBranchAddress” is that the “TTree.h” implements it as a templated method (which then internally knows the “classtype” and so on).[/quote]Yes, but the templated function are available in both and the function called in the template function implementation are virtual and overloaded in TChain …

Cheers,
Philippe.

Wile_E_Coyote · October 4, 2012, 9:49pm

Looking into the “TTree.h”, if any of the “template Int_t SetBranchAddress(const char *bname, T **add, TBranch **ptr = 0)” was called, then it would finally execute “return SetBranchAddress(bname,add,ptr,cl,type,true/false);”.
I am 100% sure that it was not the case.
Only the “TChain::SetBranchAddress(const char *bname,void *add, TBranch **ptr = 0)” was called.

pcanal · October 5, 2012, 12:42am

Hi,

Can you send me an example reproducing the problem?

Thanks,
Philippe.

Wile_E_Coyote · October 5, 2012, 8:47am

The problem is more complex than I thought.
While preparing a “trial” I noticed that TTree::SetBranchAddress" misbehaves, too.

The “trial.cxx” (requires “hsimple.root” from the “tutorials”): [code]#include “TFile.h”
#include “TChain.h”
#include “TBranch.h”
#include “TString.h”
#include “TObjString.h”

#include

void trial(void)
{
#if 1 /* 0 or 1 */
TFile file = new TFile(“hsimple.root”);
TTree tree = (TTree)file->Get(“ntuple”);
#else / 0 or 1 /
TChain tree = new TChain(“ntuple”, “ntuple”);
tree->Add(“hsimple.root”);
// tree->SetBranchStatus("", 1);
// tree->SetBranchStatus(“px”, 1);
// std::cout << "fTreeNumber = " << tree->GetTreeNumber() << std::endl;
tree->LoadTree(0); // REQUIRED
// std::cout << "fTreeNumber = " << tree->GetTreeNumber() << std::endl;
#endif / 0 or 1 */

TBranch *p = ((TBranch *)-1);
Int_t r;

Float_t px, fake_px;
Float_t *pxp = new Float_t();
Float_t *fake_pxp = new Float_t();

Int_t ix;
Int_t *ixp = new Int_t();

TString s;
TString *sp = new TString();

TObjString os;
TObjString *osp = new TObjString();

// Float_t … (should be fine)
std::cout << std::endl << "ALL should be FINE … " << std::endl;
p = ((TBranch *)-1);
r = tree->SetBranchAddress(“px”, &px, &p);
std::cout << "Float_t … " << r << std::endl;
std::cout << "p = " <SetBranchAddress(“px”, &px);
std::cout << "Float_t … " << r << std::endl;
r = tree->SetBranchAddress(“px”, pxp);
std::cout << "Float_t … " << r << std::endl;
r = tree->SetBranchAddress(“px”, &pxp);
std::cout << "Float_t … " << r << std::endl;

// Int_t … (should fail)
std::cout << std::endl << "ALL should FAIL … " << std::endl;
p = ((TBranch *)-1);
r = tree->SetBranchAddress(“px”, &ix, &p);
std::cout << "Int_t … " << r << std::endl;
std::cout << "p = " <SetBranchAddress(“px”, &ix);
std::cout << "Int_t … " << r << std::endl;
r = tree->SetBranchAddress(“px”, ixp);
std::cout << "Int_t … " << r << std::endl;
r = tree->SetBranchAddress(“px”, &ixp);
std::cout << "Int_t … " << r << std::endl;

// TString … (should fail)
std::cout << std::endl << "ALL should FAIL … " << std::endl;
p = ((TBranch *)-1);
r = tree->SetBranchAddress(“px”, &s, &p);
std::cout << "TString … " << r << std::endl;
std::cout << "p = " <SetBranchAddress(“px”, sp);
std::cout << "TString … " << r << std::endl;
r = tree->SetBranchAddress(“px”, &sp);
std::cout << "TString … " << r << std::endl;

// TObjString … (should fail)
std::cout << std::endl << "ALL should FAIL … " << std::endl;
p = ((TBranch *)-1);
r = tree->SetBranchAddress(“px”, &os, &p);
std::cout << "TObjString … " << r << std::endl;
std::cout << "p = " <SetBranchAddress(“px”, osp);
std::cout << "TObjString … " << r << std::endl;
r = tree->SetBranchAddress(“px”, &osp);
std::cout << "TObjString … " << r << std::endl;

// nonexistent branch … (should fail)
std::cout << std::endl << "ALL should FAIL … " << std::endl;
p = ((TBranch *)-1);
r = tree->SetBranchAddress(“fake_px”, &fake_px, &p);
std::cout << "nonexistent branch … " << r << std::endl;
std::cout << "p = " <SetBranchAddress(“fake_px”, &fake_px);
std::cout << "nonexistent branch … " << r << std::endl;
r = tree->SetBranchAddress(“fake_px”, fake_pxp);
std::cout << "nonexistent branch … " << r << std::endl;
r = tree->SetBranchAddress(“fake_px”, &fake_pxp);
std::cout << "nonexistent branch … " << r << std::endl;
}[/code]

Note: in the “tests” below you will see exactly which method gets called. I modified my “TTree.cxx” and “TChain.cxx” so that now every “SetBranchAddress” says that it got called (but I did not modify “TTree.h”, so templated methods will not be “verbose”).

First … “TTree” tests: [code]ALL should be FINE …
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
Float_t … 0
p = 0x8ce8018
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
Float_t … 0
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
Float_t … 0
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
Float_t … 0

ALL should FAIL …
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
Error in TNtuple::SetBranchAddress: The pointer type given “Int_t” (3) does not correspond to the type needed “Float_t” (5) by the branch: px
Int_t … -2
p = 0x8ce8018
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
Error in TNtuple::SetBranchAddress: The pointer type given “Int_t” (3) does not correspond to the type needed “Float_t” (5) by the branch: px
Int_t … -2
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
Error in TNtuple::SetBranchAddress: The pointer type given “Int_t” (3) does not correspond to the type needed “Float_t” (5) by the branch: px
Int_t … -2
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
Error in TNtuple::SetBranchAddress: The pointer type given “Int_t” (3) does not correspond to the type needed “Float_t” (5) by the branch: px
Int_t … -2

ALL should FAIL …
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
TString … 0
p = 0x8ce8018
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
TString … 0
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
TString … 0

ALL should FAIL …
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
TObjString … 0
p = 0x8ce8018
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
TObjString … 0
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
TObjString … 0

ALL should FAIL …
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
Error in TNtuple::SetBranchAddress: unknown branch -> fake_px
nonexistent branch … -5
p = 0xffffffff
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
Error in TNtuple::SetBranchAddress: unknown branch -> fake_px
nonexistent branch … -5
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
Error in TNtuple::SetBranchAddress: unknown branch -> fake_px
nonexistent branch … -5
TTree::SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, Bool_t)
Error in TNtuple::SetBranchAddress: unknown branch -> fake_px
nonexistent branch … -5[/code] Note the following problems:

As soon as a “real class” is involved (TString of TObjString), it happily returns 0 (should be -2 in all these crazy cases)!
In case of a non-existent branch, the pointer “p” is left uninitialized (should be set to 0)!
The good news is that both, the interpreted code and the (ACLiC pre-)compiled code, behave the same.

Then … “TChain” tests: [code]ALL should be FINE …
TChain::SetBranchAddress(const char , void, TBranch**)
Float_t … 0
p = 0x937b280
TChain::SetBranchAddress(const char , void, TBranch**)
Float_t … 0
TChain::SetBranchAddress(const char , void, TBranch**)
Float_t … 0
TChain::SetBranchAddress(const char , void, TBranch**)
Float_t … 0

ALL should FAIL …
TChain::SetBranchAddress(const char , void, TBranch**)
Int_t … 0
p = 0x937b280
TChain::SetBranchAddress(const char , void, TBranch**)
Int_t … 0
TChain::SetBranchAddress(const char , void, TBranch**)
Int_t … 0
TChain::SetBranchAddress(const char , void, TBranch**)
Int_t … 0

ALL should FAIL …
TChain::SetBranchAddress(const char , void, TBranch**)
TString … 0
p = 0x937b280
TChain::SetBranchAddress(const char , void, TBranch**)
TString … 0
TChain::SetBranchAddress(const char , void, TBranch**)
TString … 0

ALL should FAIL …
TChain::SetBranchAddress(const char , void, TBranch**)
TObjString … 0
p = 0x937b280
TChain::SetBranchAddress(const char , void, TBranch**)
TObjString … 0
TChain::SetBranchAddress(const char , void, TBranch**)
TObjString … 0

ALL should FAIL …
TChain::SetBranchAddress(const char , void, TBranch**)
Error in TChain::SetBranchAddress: unknown branch -> fake_px
nonexistent branch … -5
p = 0
TChain::SetBranchAddress(const char , void, TBranch**)
Error in TChain::SetBranchAddress: unknown branch -> fake_px
nonexistent branch … -5
TChain::SetBranchAddress(const char , void, TBranch**)
Error in TChain::SetBranchAddress: unknown branch -> fake_px
nonexistent branch … -5
TChain::SetBranchAddress(const char , void, TBranch**)
Error in TChain::SetBranchAddress: unknown branch -> fake_px
nonexistent branch … -5[/code]Note the following problems:

It happily returns 0 for any type of pointer, if the requested branch exists!
(The good news is that it sets the pointer “p” to 0 in case of a non-existent branch.)
Again, both, the interpreted code and the (ACLiC pre-)compiled code, behave the same.

Wile_E_Coyote · October 5, 2012, 11:35am

I have introduces some more crazy “pointer type” tests … previous post modified (get the “trial.cxx” source code again, if you’ve already done it before).

pcanal · October 5, 2012, 5:41pm

[quote]2. In case of a non-existent branch, the pointer “p” is left uninitialized (should be set to 0)![/quote]This is intentional. The error is signaled by the return value and the branch pointer is less unchanged.

Cheers,
Philippe.

pcanal · October 5, 2012, 5:42pm

HI,

[quote]1. As soon as a “real class” is involved (TString of TObjString), it happily returns 0 (should be -2 in all these crazy cases)![/quote]I see. I will fix this.

Cheers,
Philippe.

Wile_E_Coyote · October 5, 2012, 6:20pm

[quote=“pcanal”][quote]2. In case of a non-existent branch, the pointer “p” is left uninitialized (should be set to 0)![/quote]This is intentional. The error is signaled by the return value and the branch pointer is less unchanged.[/quote] Then the “TTree” behaviour is inconsistent with respect to the “TChain” (here the “p” is set to 0, and I seem to like it).

pcanal · October 5, 2012, 6:39pm

[quote]1. As soon as a “real class” is involved (TString of TObjString), it happily returns 0 (should be -2 in all these crazy cases)![/quote]is fixed in the repository.

Thanks,
Philippe.

pcanal · October 5, 2012, 7:38pm

[quote]Then the “TTree” behaviour is inconsistent with respect to the “TChain” (here the “p” is set to 0, and I seem to like it).[/quote]Yes, the behavior was inconsistent. TTree now also set the pointer to 0.

Thanks,
Philippe.

Wile_E_Coyote · October 5, 2012, 8:17pm

The “set pointer to 0” fix is not in the v5-34-00-patches branch?

P.S. I can confirm that the new TTree::SetBranchAddress gives “proper” error messages and return codes now (the TBranch::SetBranchAddress still returns 0 all the time).

pcanal · October 5, 2012, 8:59pm

Hi,

[quote]The “set pointer to 0” fix is not in the v5-34-00-patches branch?[/quote]It is now there

Philippe.

[quote]P.S. I can confirm that the new TTree::SetBranchAddress gives “proper” error messages and return codes now (the TBranch::SetBranchAddress still returns 0 all the time)[/quote]See root.cern.ch/viewvc/trunk/root/t … t&view=log

Operation: SetBranchAddress

svn diff [code]Index: tree/tree/src/TChain.cxx

#pragma link C++ function operator+(const TCut&, const char*); #pragma link C++ function operator+(const char*, const TCut&); Index: tree/tree/inc/TTree.h

#pragma link C++ function operator+(const TCut&, const char);
#pragma link C++ function operator+(const char, const TCut&);
Index: tree/tree/inc/TTree.h