Hi Kris et al, On Thu, 09 Mar 2000 12:31:58 -0600 hagel@comp.tamu.edu wrote: > Hi Christian (and all) > The BrEvent you are talking about was purposely designed as a general way > to handle BRAHMS data at all of the intermediate stages. That is how we > came upon the idea of the EventNode which can in turn have lists of data > elements as well as other event nodes (they all inherit from > BrDataObject). Yeah, that's fine, but ... (I'll come back to this later on) > In fact, it is completely trivial to put the BrEvent into a root tree. > That is what is done in BrEventIO. The BrEvent's are put into trees and > written into the files and read back as such. It is a straight trivial > root operation. Hmm .... In BrEventIO, it's trivial, true. But the tree you create is somewhat in-user friendly. You see, the nice thing about TClonesArrays, is that they show up as folders in a TBrowser, and hence make the data structure more transparent to the user. Also, It'd be nice to have more then one super-TBranch, since again, this makes the structure more transparent, and there are some nice features using these TBranches in ROOT. To ignore these features is almost the same as reverting to PAW NTuples! > These trees are not, however, the kind of root trees that > the root developers envisioned. We knew that from the beginning and our > idea was that other data structures would be developed later that would > have real physics meanings that would be used in root trees for the > purposes of looking at data. These structures would be more specific and > necessarily not as general as the BrEvents we have. True, BrEvent is very general, but I see no great obsticale in having a similar generallity in a BrTreeEvent class. For example, instead of adding new BrEventNodes, one could add new TBranches, and instead of putting BrDataTables into these BrEventNodes, we'd put objects of classes with internal TClonesArrays, or even TObjArrays (like a BrDataTable), if that is called for. > On BrEvent and BrEventNode, I was not aware that we converted an internal > TObjArray into a THashTable. This might be left over from the fact that > we got this code from Phobos which had a THashTable and we converted it to > a TObjArray. Since the code appeared to work, we did not notice the > error. I dont't know if this is an error or not, but at least it looked strange to me. > BrRawDataInput: I agree completely that the table names should not be > hard coded into methods. I mentioned in a previous mail that I had begun > the process of removing these hard codes and in fact there is already an > include file called BrTableNames.h or something like that. I will make > that my highest priority in the next days between our beamtime and submit > it into the repository. As far as the scheme, I needed to match the names > that were in the other parts of the code (also hard coded!!!) We had a > scheme which is documented and on our web page somewhere, but this scheme > has probably evolved over time. Anyway, the hard coded stuff should be > removed and then one can worry about scheme once all of the names are in > the same file. I know you've mentioned this BrTableNames.h before (on December 15), and honestly thought this was intergrated well by now. I know your busy, so don't take it the wrong way - I too know how it is, you want to have 48 hours on a day, but ...! > As far as the Decode methods being too large, I think that is a matter of > taste. When I put it together, I thought I had made them about the right > size. To divide even more led me to think I might be duplicating some > code, but I will revisit that and see if that is still valid. Why I say they are too large: Printed on A4 paper (in a 10pt font), they take up about 3 pages on the average. That's one page too much. Also, splitting up the methods, would increase the readability tremendiously. And if you use the BrException classes, the code could shrink even furhter. As far as duplication goes, I don't think that'll be a great problem. In fact, I don't think you'll duplicate more then is already done. And a suggestion for the TPC cases: Instead of: switch(recordId) { ... case 2302: case kT1B: case kT1: case kT2B: case kT2: if(recordId==kT1B){ fTable = new BrDataTable("TPCSequence T1B","Raw mapped"); ... else if(recordId==kT1){ fTable = new BrDataTable("TPCSequence T1", "Raw mapped"); ... else if(recordId == kT2B){ fTable = new BrDataTable("TPCSequence T2B","Raw mapped"); ... else if(recordId == kT2){ fTable = new BrDataTable("TPCSequence T2", "Raw mapped"); ... else if(recordId == 2302){ fTable = new BrDataTable("TPCSequence T1B","Raw mapped"); .... } What about doing: switch(recordId) { ... case 2302: case kT1B: DecodeTPC(kT1BName); break; case kT1: DecodeTPC(kT1Name); break; case kT2B: DecodeTPC(kT2BName); break; case kT2: DecodeTPC(kT2Name); break; ... } And DecodeTPC(const Char_t* name) { if(fRawEvent->getRecordUint16(recordId, buf16, length) != fRawEvent->kOk) ... const Char_t tablename = Form(kTPCFormat, name); BrDataTable* fTable = new BrDataTable(tablename, kTPCTitle); fFFSNode->AddDataTable(fTable); ... } where const Char_t* kT1BName = "T1B" ... const Char_t* kTPCFormat = "TPCSequence %s"; ... const Char_t* kTPCTitle = "RAW mapped"; > That's my one cent response to the two pennies worth. Hmm, I guess we're down to lires! Cheers, Christian ----------------------------------------------------------- Holm Christensen Phone: (+45) 35 35 96 91 Sankt Hansgade 23, 1. th. Office: (+45) 353 25 305 DK-2200 Copenhagen N Web: www.nbi.dk/~cholm Denmark Email: cholm@nbi.dk
This archive was generated by hypermail 2b29 : Thu Mar 09 2000 - 14:36:54 EST