Hi Jens Ivar, Flemming, Kris, et al, [Ok, this became a rather long mail - sorry. Jens Ivar, Flemming and Kris I recommend read it. The rest in case you want some more in-depth information on BRAT and why I objected to some of Jens Ivars recent additions.] On Jens Ivar Jordre <jens@fi.uib.no> Re: BRAT-2-0-27 wrote concerning "Thu, 16 Aug 2001 23:48:00 -0100 (GMT+1)": I looked a bit closer at your code yesterday, and have some more comments. > > > BrSpectrometerTrackingPackage Base class of the 2 clases above > > > > Is there any good reason for a base class here? Most packages are so > > extremly simple, that having a base class is in most cases ludicrous. > > You may be right about this. My plan was to elaborate on an interface as > similar as possible for MRS and FFS, so the interface was to go into > BrSpectrometerTrackingPackage. I'll remove this one for now, but I don't > guaranty I'll not introduce it later. :) Well, I don't mind the class BrSpectrometerTrackingPackage - tough it's a bloody long name - so if you want to leave I have no real problem with that. Hmm, maybe I made it a bit to easy to make new modules/packages by introducing the Elisp function 'brat-class' :-) > > > BrGeometryManager Manager of geometry nodes, for > > > visualisation > > > > Watch it. We need to have a real geometry DB up an running soon, and > > to not confuse things to much, I strongly recommend you change the > > name of that class to something that people will not see as a > > analysis/simulation geometry manager, say BrVizGeomManager. Why do > > you need this manager in the frist place? I could rather imagine > > having classes like in GEANT4, where you have something like: > > > > G4LogicalVolume: Knowns size, position, rotation and mother volume, > > as well as how to draw it self in the mother volume. > > G4PhysicalVolume: Derives from LogicalVolume. Has additional > > information of material (defraction indicies, dE/dx, > > and so on). > > G4Detector: Contains LogicalVolume objects. Also knows how to > > draw hits, tracks and so on in those volumes. > > > > In BRAT terms, we could have something like a class > > > > BrVizModule > > > > That essentially is like the G4Detector class, but in the BRAT-style: > > > > Init Draws detector in mother volume (CAVE, FFS, BFS, MRS) > > Begin Redraws if needed > > Event Draws hits, tracks, etc. > > > > The Viz is short for "visualisation" [en_BR] or "visualization" in > > [en_US] > > I'm using this class to process macro files generated from GEANT > geometry using g2root. Uh! g2root does a full conversion of the GEANT3.21 [1] volume/material/position/rotation common blocks (sigh) which is far far too much for an event display. See also comments to Flemmings mail at the end of this mail. I guess the points that Flemming make about Rene and me (?) being obsessed with GEANT style geometry goes double for all of this, since you use g2root! > This class allows for easy access to different nodes (TNode) in the > BRAHMS geometry. Since I use/plan to use it in several classes I > found it easier to make it a manager than passing it as argument to > the classes using it. I would also like it to be a singleton so that > changes applied to it in one class shows up over all. I have found > it usefull. You should rather define the volumes in the detector specific classes, using the service of BrGeometryDbManager. Also note, that ROOT has already implemented the behaviour you're addressing via globals gGeomtry [2]. Hmm. All in all, I think all this belongs in a seperate package from BRAT, since it's not likely that the code will be used outside of the event display. > > > BrTriggerManager Easy managing of trigger mask > > > > You know that there's a BrTriggerFilter in BRAT already, don't you? > > It has been evaluated by a great many people and found sound. I don't > > see the need for an addtitional manager. Please elaborate on the > > details of this. > > Ok, elaboration starts. (Do go and get a cup of coffee, i.e. an espresso > as the elaboration is not long enough for a tall latte.) If only we had an espresso machine here at NBI (not like at CERN where there's one at every corner!). > a) The same argument as ... I guess you pass a given BrEvent through the regular BRAT analysis modules to do tracking, and ya di da. Then, you might as well pass it trough a BrTriggerFilter. Also, I see absolutly no reason why any analysis class needs access to the trigger words, assuming you have filtered out the proper events first. We (Flemming, Kris, and I at least) had a long discussion on that some time ago, and the consequence of that was that BrEventManager was depriciated for exactly these reasons. > > > BrEventInputManager Easy input of new events Well, it's really a cache isn't it? Also, you could easily combine the services of BrEventList and BrEventInputManager into a module or something, say BrCacheEventModule and use it with bratmain as: BrMainModule* mainModule = new ... BrIOModule* inputModule = new ... mainModule->AddModule(inputModule); BrTriggerFilter* triggerFilter = new ... mainModule->AddModule(triggerFilter); BrCacheEventModule* cacheModule = new ... cacheModule->SetMaxEvents(10); mainModule->AddModule(cacheModule); Having said that, I think you dhould rather be using as input a TTree with the relevant branches, since it's much easier to go back and forth in such a file(s). Ofcourse that would mean that your event displayer should be segregated out of BRAT proper! > Ok, I get the point. My fault and typo. It is of course a FIFO. :) I'll > update. It could a have been a LIFO, though a FIFO is probably what you want. I was meerly curious. > Some time one would typically like to use the event list, other times not. > By using the event input manager, one can can have the same interface > whether one chooses to use the list or not. I made it a manager to have it > as a singleton. There's a general comment to all of the above that is probably the best argument for segregating out the event displayer: You additions could load to a fork of the BRAT code base in the worst case, or a fork in practise at the least. That is, someone will start doing things the BrEventInputManager way, and others will use the BrMainModule way, quickly leading to incompatible code, analysis and so on - not a nice situation. Much of the effort that want in to BRAT the last couple of years has been to provide us with a homogenious toolkit for doing analysis. The advent of bratmain is IMHO the culmination of this (I'd like to stress that bratmain is not altogther my idea - I borrowed heavily from AliROOT [3]). Your general idea is not at all unsound. In fact, it's close to what other toolkits, like AliROOT [3], AtlFast [4], and I believe Gaudi [5] too, do. Were I to start a new toolkit for a detector today, some of your ideas would be at the core of things. However, it's not how BRAT was designed. The main carrier of information in BRAT is the BrEvent. Modules expect to be feed that information. In a way, BrEvent is the pipe (with all it's contents) that make up the BRAT pipeline; Modules tap into that pipeline and may add to the contents. In a sense, the modules are the passive components of BRAT. Flemming once coined the term that BRAT is "event-driven". This is opposed to a design like AliROOT [3], where the "modules" are much more active. The are in charge of the data, and can at their leasure make it avaliable to other "modules" via a common "message board". This design has some advantages (I believe it's easier to parallise) over the BRAT design, and ofcourse some weaknesses too. The lesson of all this is: Take out those classes above that has to do with event visualisation and put in a seperate package, and maybe you should rethink the strategy and use a TTree rather than a list BrEvents as you cache. > > > BrModule > > > As many decendants of BrModule create subdirectories of gDirectory for > > > their histograms, BrModule::Book now checks if the first newly created > > > object is a TGDirectory. If it is it adds histograms from it to > > > fHistograms. > > > > This is _not_ good. This means that we have to link the BRAT........... > > Yes, new typo. Reading your mail the first time I could not understand why > you brought GUI stuff into the discussion. Then I discovered it. Instead > of TGDirectory I meant of course TDirectory. No GUI allowed here!! The same argument goes for the dependence on TNode, TGeometry and so on. In short, remove all classes that does graphics (not just GUI) from BRAT. And now to Flemmings email: On "Flemming Videbaek" <videbaek@sgs1.hirg.bnl.gov> Geometries wrote concerning "Thu, 16 Aug 2001 22:38:30 -0400": > Let me add my 5 cents (44 oere) worth on geometry. > There seem to be an obcession both by Rene and now by Christian to > use a geant like geometry description - with geant like > mother/daugther volumes, materials etc. I cannot answer for Rene, but I personally have a hard time imagining a better strategy for geometry descriptions than the mother-daughter approach. Consider the BRAHMS experiment: We have essentially one mother volume: the cave it self. In side the cave we have 3 major ('older') daughter volumes: FFS, BFS, and MRS; plus 5 minor ('younger') daughter volumes: ZDCL, ZDCR, BBL, BBR and MULT. In the 3 major daughters, we find more daughter volumes. What is the best way to describe the position of these grand-daughters to the cave? Well, since the 3 older daughters can be moved around, and the grand-daughters with them, it makes perfect sense to describe the position and volumes of the grand-daughters in terms of the older daughters. As Valeri Fine (of STAR) once pointed out to me (in private communique spawned by [6]) geometry descriptions are really functional descriptions, like "this volume is 2 times shorter than that one", which is best captured by a mother-daughter (or containment) description. Of course, if a useful functional programming language [7] was avaliable to the scientific community, that would probably the choice for geometry descriptions. Similar arguments goes for "onion" detectors. > This has very little to do with the geometry that is needed to do > particle tracking, In the sense of tracking in a single volume like a TPC or a DC, you're right. > hit matching and even event displays. Well consider the alternative: Everything is described in terms of it's global position in the cave. That, I believe you will agree, is down right stupid, since the functional description that so easily propegates changes are completly gone. Yes, you gain the immediate benefiet that all hits and tracks coordinates are readily avaliable in the same reference system. However, this is after all easily facilitated by (linear) transformations (translations and orthogonal rotations). > Speaking from a long experience with geant except for very simple > and unrealistic detectors that description is way to convoluted for > the first and primary purpose. Well, one of the reasons why it was so difficult to deal with GEANT 3.21 [1] was the choice of Fortran 4 and later Fortran 77 as the language of implementation. Now, I'm not too keen on some of the design choices in GEANT 4 [8], but one thing they after all managed to make more transparent is the geometry description. Recent development of AliROOT [3] and releated projects [9] have shown that once you adopt an Object Oriented Paradigm, the once so tricky buissness of geometry descriptions become rather easy. This ofcourse has to do with inheritance and polymorphism. In fact, the TGeant3 project [9] may very well spawn some generalised geometry description useful for all kinds of detector visualisation. > This is the main reason for a much simpler dependence of the active > areas of detectors being described relative to just a or two-level > one-level up description of most of the brahms detectors, that Kris > has implemented very recently, and which I trust will do the task > for us. I'm not saying (as you seem to think [10]) that a full-fleshed geometry description like the one used for the GEANT 3.21-based simulation package BRAG is what we want in BRAT - a watered-down version should suffice. However, it's clear that the geometry description of both BRAT and BRAG should be close to each other, so as to easy the concurrency of the two. Again, the mother-daughter approach is in fact what you want: We do tracking in the grand-daughter volumes, independent of the other grand-daughters, and then we do another tracking pass in the older daughter volumes, also independent of the other older daughter volumes. Only when we need to track globally, do we need all the information on the older daughters, but then we are tracking in the mother volume, so that information is readily avaliable. As I said, I have a hard time imagining a better design than the mother-daughter design. Ok, so I may be ignorant on this, and if anyone has compelling arguments as to why the mother-daughter approach should be replaced, I'd very much like to hear them. Finally, I have no doubt that the geometry DB will work out fine. Flemming, Kris and I have had long discussions on this the past 6 months, and Flemmings respond above is probably motivated by some of my contributions to that discussion. Suffice to say that I advocated a more abstract approach than what Kris initially proposed. The reasons for insisting on abstraction should be clear to anyone that has developed code for BRAT, and I don't want to tire you with an additional 300 lines of stuff that you probably already know. Christian Holm Christensen ------------------------------------------- Address: Sankt Hansgade 23, 1. th. Phone: (+45) 35 35 96 91 DK-2200 Copenhagen N Cell: (+45) 28 82 16 23 Denmark Office: (+45) 353 25 305 Email: cholm@nbi.dk Web: www.nbi.dk/~cholm Notes ---------------------------------------------------------------------- [1] http://wwwinfo.cern.ch/asd/geant/index.html [2] http://root.cern.ch/root/html/TGeometry.html [3] http://alisoft.cern.ch/offline/ [4] http://root.cern.ch/root/Atlfast.html [5] http://proj-gaudi.web.cern.ch/proj-gaudi/ [6] http://root.cern.ch/root/roottalk/roottalk01/0121.html [7] http://www.foldoc.org/foldoc/foldoc.cgi?functional+programming [8] http://wwwinfo.cern.ch/asd/geant4/geant4.html [9] http://www-root.fnal.gov/root2001/presentations/session6/2001_06_14_tgeant.pdf [10] Now that we're in the buisness of guessing the contents of each others minds, I don't feel that terrible about this remark :-)
This archive was generated by hypermail 2b30 : Fri Aug 17 2001 - 08:00:39 EDT