TClonesArray

Philippe,

I have no idea what you were doing, but it doesn’t work here.
Again:
Remove the first Clear():
–> [quote]Traceback (most recent call last):
File “bug.py”, line 22, in ?
print iB.gamma.uid
AttributeError: ‘TObject’ object has no attribute ‘gamma’
[/quote]
Put it back in and remove the line print j.bList[0].gamma.uid
–> It doesn’t run over the list, because Clear() causes GetEntries() to return 0.

Put the line back in. Suddenly the list has a different length ? I don’t think so !

Are you saying you cannot reproduce this ?
Jan

[quote]I have no idea what you were doing, but it doesn’t work here.[/quote]simply running the code as is … Now that you remind me what the problem now was … I can reproduce some of what you say.
However I have no clue what you mean by:

Anyway at the end you currently have: if j.bList.GetEntries() > 2: j.bList.RemoveAt(1) j.bList.Clear()which removes one element and then remove all the others elements. The correct calls are indeed if j.bList.GetEntries() > 2: j.bList.RemoveAt(1) j.bList.Compress()and you should not (in your specific case) use Clear at all.

Now … this all works in the C++ version of your code. In the python version, I still get:[quote]Event: 2
3 entries
3 entries
10041
10041
10041
Traceback (most recent call last):
File “bug.py”, line 24, in ?
print iB.gamma.uid
AttributeError: ‘TObject’ object has no attribute ‘gamma’
[/quote]I am not sure yet why this is happening, hopefully Wim can shed some light.

Cheers,
Philippe.
bug.py (794 Bytes)
bug.C (917 Bytes)

Hi Philippe,
now I am confused. Rene told me:

[quote=“brun”]You should never call TClonesArray::Compress while writing a Tree (or you have to set addresses again).
At the end or beginning of each event, call TClonesArray::Clear

Rene[/quote]
Now you are saying I should call Compress() rather than Clear(). (Which is also what Wim told me)
Obviously both can’t be true…

What I mean by [quote]Put the line back in. Suddenly the list has a different length ? I don’t think so !
[/quote]
is that after Clear() a loop over the list (of course) doesn’t process anything, because GetEntries() returns 0.
However, calling Clear() and then accessing an entry in the list and then looping over the list does process one entry. I was very surprised by this.

Cheers,
Jan

[quote]Now you are saying I should call Compress() rather than Clear(). (Which is also what Wim told me)
[/quote]Rene’s advice is a general advice that does not fit your particular situation (trying to copy a TClonesArray from one tree to another).
Clear is explictly not what you are looking for since it empties the collection (but due to the intentional memory optimization it does not run the destructor nor free up the memory).

[quote]is that after Clear() a loop over the list (of course) doesn’t process anything, because GetEntries() returns 0.
However, calling Clear() and then accessing an entry in the list and then looping over the list does process one entry. I was very surprised [/quote]This is due to the interface of TClonesArray. Remember that usually the first initialization of TClonesArray element is suppose to be:new (clones[index]) MyClass(...);the ‘clones[index]’ induces the TClonesArray object to allocate the proper memory (but not initiliazed) and mark the ‘index’ entry to be valid. So if you doclones.Clear(); clones[index];You re-tell the clones array that clones[index] is valid but without re-initializing it.

Cheers,
Philippe.

PS. With the C++ code I sent you I get files that looks like what I expect:[code]root [0] TFile *_file0 = TFile::Open(“x.root”)
root [1] bugMe->Scan(“bList.chi2”)


  • Row * Instance * bList.chi *

  •    0 *        0 * 463.71948 *
    
  •    0 *        1 * 0.0154493 *
    
  •    0 *        2 * 0.0154493 *
    
  •    0 *        3 * 0.0154493 *
    
  •    0 *        4 * 0.0154493 *
    
  •    1 *        0 * 1.2567778 *
    
  •    1 *        1 * 1.3962125 *
    
  •    1 *        2 * 2.0331127 *
    
  •    2 *        0 * 33.346118 *
    
  •    2 *        1 * 33.159679 *
    
  •    2 *        2 * 2037.6967 *
    
  •    3 *        0 * 13.120962 *
    
  •    3 *        1 * 43.959659 *
    
  •    3 *        2 * 7.8385129 *
    
  •    4 *        0 * 78.134170 *
    
  •    4 *        1 * 1.8914668 *
    
  •    4 *        2 * 837.13458 *
    

***********************************[/code]becomes

[code]root [0] TFile *_file0 = TFile::Open(“xPy.root”)
root [1] bugMe->Scan(“bList.chi2”)


  • Row * Instance * bList.chi *

  •    0 *        0 * 463.71948 *
    
  •    0 *        1 * 0.0154493 *
    
  •    0 *        2 * 0.0154493 *
    
  •    0 *        3 * 0.0154493 *
    
  •    1 *        0 * 1.2567778 *
    
  •    1 *        1 * 2.0331127 *
    
  •    2 *        0 * 33.346118 *
    
  •    2 *        1 * 2037.6967 *
    
  •    3 *        0 * 13.120962 *
    
  •    3 *        1 * 7.8385129 *
    
  •    4 *        0 * 78.134170 *
    
  •    4 *        1 * 837.13458 *
    

***********************************[/code]

Hi,

Actually there was a problem in the interaction of RemoveAt and the rest of the code (the object was destructed but never (re)constructor when the memory needed to be reuse. This is fixed in the CVS repository and will be part of the upcoming release.

Cheers,
Philippe.

Jan,

[summing up the private e-mails for reference]

calling the non-const version of operator of TClonesArray caused uninitialized memory to be allocated and made available for the use of the placement new operator. This is how TClonesArray is efficiently used from C++, but that behaviour isn’t going to do any good when used from python. Note that the operator is sometimes called implicitly by the interpreter during iteration, explaining the funny side effects that you observed.

Added to CVS (and in time for the upcoming pro release this week), code to enforce the use of At() (from TObjArray) for getitem, such that it’ll always be memory safe to index into the TClonesArray (this is the equivalent of the const version of operator[]).

As for assignment: assigned to the return TObject*& can’t be done since it is uninitialized memory, hence operator=() won’t behave properly. Instead, setitem for TClonesArray has been reimplemented, but it has to be used with a little care: only temporaries will work naturally, as the assignment takes over ownership (as is normal in using TClonesArray).

For example:

[code] >>> c = TClonesArray( “MyClass” )

c[ 0 ] = MyClass()
c[ 1 ] = MyClass()
m = MyClass()
c[ 1 ] = m # safe: will call ~MyClass() on the old one
c[ 1 ] = MyClass() # m is now destroyed (None)
[/code]
Whereas removal works with del, as usual: >>> del c[1]
Needless to say that all the above comes with a price (the creation of the temporary negates the original point of the TClonesArray, which is to prevent allocating/freeing many little objects) and thus it is usually better to simply copy the contents (this is copy by reference) of a TClonesArray into a list:

Then modify to your heart’s content (sorting and re-arranging, as you do in your examples is what lists are for, and it all happens by reference, so it is fast), then copy copies of the elements back to the TClonesArray for writing out to disk:

 >>> for i in range(len(l)):
 >>>    c[i] = MyClass( l[i] )

And delete any remainder as necessary. Note that you can also make copies into the list first, then Clear() the TClonesArray and finish up by copying back into the array like normal.

The above will be added to the documentation as well.

Best regards,
Wim

Hi,

This seems to be related to the problem I am facing and wanted to see if anyone could chime in on if this is related.

I’m looking to build a tree from a number of objects, lets say multiple channels from a DAQ for simplicity, these channels don’t always fire and so I won’t always have data from them for an event. In the past I’ve stuck them in vectors but then the random access index just gives me the point in which they were inserted. I’d rather have the index give me the channel number then I can plot data[0]:data[1] , for example. Of course I could use an array, but I haven’t found a good solution to make an array of structs or classes, as my data is not just one dimensional for each channel.

I thought TCloneArray was the solution. I build an array and pack it with an object at each index corresponding to a channel then set the values as the data comes in. Unfortunately, the data from the last event is left in the array so I assumed TClonesArray::Clear() would set the classes back to the initial values (there is some magic occurring here that is not clear to me. Does this make calls to the Clear function of the object in the array or just delete them from memory? How is Clear and Delete different?). This seems to work as the values for the non-firing channels vanish in the tree, but then the code seg faults because an event has only one channel less than previous and the TClonesArray is smaller. I then found this thread where Compress was suggested, and this resolves the seg fault. Unfortunately, the data is still not organized in the tree as I expected, the index is not giving me zero for channels that were not set.

I’ve attached a short example where I use a TLine object for storage (It has four floating point values I could set easily.) I’ve set fY1 to the channel number, fY2 to the event number, and fX1 and fX2 to some random gaussian distribution. I want to plot fX1[0]:fX1[1], but using Scan I see that fY1[0] takes on values of both 0 and 1 although I expected only 0.

root [1]
root [1] t->Scan("fX1:fX2:fY1:fY2:fY1[0]")
***********************************************************************************
*    Row   * Instance *       fX1 *       fX2 *       fY1 *       fY2 *    fY1[0] *
***********************************************************************************
*        0 *        0 * 9.5652356 * 20.781796 *         0 *         0 *         0 *
*        1 *        0 * 99.943282 * 399.09912 *         1 *         1 *         1 *
*        2 *        0 * 99.589236 * 401.39119 *         1 *         2 *         1 *
*        3 *        0 *           *           *           *           *           *
*        4 *        0 *           *           *           *           *           *
*        5 *        0 *           *           *           *           *           *
*        6 *        0 *           *           *           *           *           *
*        7 *        0 * 99.263970 * 400.57972 *         1 *         7 *         1 *
*        8 *        0 * 99.184194 * 399.59305 *         1 *         8 *         1 *
*        9 *        0 *           *           *           *           *           *
*       10 *        0 * 99.500360 * 399.81756 *         1 *        10 *         1 *
*       11 *        0 * 101.99686 * 400.00480 *         1 *        11 *         1 *
*       12 *        0 *           *           *           *           *           *
*       13 *        0 *           *           *           *           *           *
*       14 *        0 *           *           *           *           *           *
*       15 *        0 * 99.863690 * 399.80078 *         1 *        15 *         1 *
*       16 *        0 *           *           *           *           *           *
*       17 *        0 *           *           *           *           *           *
*       18 *        0 * 100.02206 * 399.92874 *         1 *        18 *         1 *
*       19 *        0 *           *           *           *           *           *
*       20 *        0 * 99.667622 * 399.75272 *         1 *        20 *         1 *
*       21 *        0 *           *           *           *           *           *
*       22 *        0 *           *           *           *           *           *
*       23 *        0 * 101.44492 * 399.44861 *         1 *        23 *         1 *
*       24 *        0 * 11.338374 * 20.488170 *         0 *        24 *         0 *

Sorry for the long discussion, I’m just not sure how to shorten the problem any more without losing the clarity.
TClonesArrayWrite.cpp (1.16 KB)