From 0e458701f0498f02e5f8878916a352a1a47a2dbd Mon Sep 17 00:00:00 2001 From: Robert Schoefbeck Date: Wed, 8 Mar 2017 10:21:54 +0100 Subject: [PATCH 1/4] removing unused naming functionality in getEventList --- core/python/Sample.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/core/python/Sample.py b/core/python/Sample.py index 49dbb47..5e4f622 100644 --- a/core/python/Sample.py +++ b/core/python/Sample.py @@ -430,7 +430,7 @@ def __loadChain(self): else: logger.error( "Check of root file failed. Skipping. File: %s", f ) except IOError as e: - logger.warning( "Could not load file %s", f ) + logger.error( "Could not load file %s", f ) raise e logger.debug( "Loaded %i files for sample '%s'.", counter, self.name ) @@ -525,7 +525,7 @@ def combineWithSampleWeight(self, weightString): self.name, weightString ) return weightString - def getEventList(self, selectionString=None, name=None): + def getEventList(self, selectionString=None): ''' Get a TEventList from a selectionString (combined with self.selectionString, if exists). ''' @@ -536,12 +536,7 @@ def getEventList(self, selectionString=None, name=None): self.chain.Draw('>>'+tmp, selectionString_ if selectionString else "(1)") elistTMP_t = ROOT.gDirectory.Get(tmp) - if not name: - return elistTMP_t - else: - elistTMP = elistTMP_t.Clone(name) - del elistTMP_t - return elistTMP + return elistTMP_t def getYieldFromDraw(self, selectionString = None, weightString = None): ''' Get yield from self.chain according to a selectionString and a weightString From 14f3fdc729184b4400d5e8cf745b5774173558c4 Mon Sep 17 00:00:00 2001 From: Robert Schoefbeck Date: Wed, 8 Mar 2017 10:28:25 +0100 Subject: [PATCH 2/4] fix bug when running over a small number of events. More care with event lists --- core/python/LooperBase.py | 2 +- core/python/Sample.py | 2 +- core/python/TreeMaker.py | 10 +++- core/python/TreeReader.py | 111 +++++++++++++++----------------------- 4 files changed, 55 insertions(+), 70 deletions(-) diff --git a/core/python/LooperBase.py b/core/python/LooperBase.py index 25a655d..b6ca82c 100644 --- a/core/python/LooperBase.py +++ b/core/python/LooperBase.py @@ -34,7 +34,7 @@ def __init__(self, variables): # Internal state for running self.position = -1 - self.eList = None + self._eList = None self.classUUIDs = [] diff --git a/core/python/Sample.py b/core/python/Sample.py index 5e4f622..59a1e93 100644 --- a/core/python/Sample.py +++ b/core/python/Sample.py @@ -532,7 +532,7 @@ def getEventList(self, selectionString=None): selectionString_ = self.combineWithSampleSelection( selectionString ) tmp=str(uuid.uuid4()) - logger.debug( "Making eList for sample %s and selectionString %s", self.name, selectionString_ ) + logger.debug( "Making event list for sample %s and selectionString %s", self.name, selectionString_ ) self.chain.Draw('>>'+tmp, selectionString_ if selectionString else "(1)") elistTMP_t = ROOT.gDirectory.Get(tmp) diff --git a/core/python/TreeMaker.py b/core/python/TreeMaker.py index d7d9e36..7a7dc3e 100644 --- a/core/python/TreeMaker.py +++ b/core/python/TreeMaker.py @@ -33,6 +33,12 @@ def __init__(self, variables, sequence = [], treeName = "Events"): # function to fill the event self.sequence = sequence + def debugBranchAddresses( self, prefix = ""): + ''' If strings are empty, there is an issue with memory. Only for debugging purposes. + ''' + for b in self.branches: + print prefix, b.GetName(), repr(b.GetAddress()) + def cloneWithoutCompile(self, externalTree = None): ''' make a deep copy of self to e.g. avoid re-compilation of class in a loop. Reset TTree as to not create a memory leak. @@ -54,6 +60,7 @@ def cloneWithoutCompile(self, externalTree = None): res.tree = ROOT.TTree( treeName, treeName ) res.makeBranches() + #self.debugBranchAddresses(prefix = "cloneWithoutCompile") return res @@ -81,7 +88,7 @@ def makeBranches(self): vectorCount+=1 else: raise ValueError( "Don't know what variable %r is." % s ) - + #self.debugBranchAddresses(prefix = "makeBranches") logger.debug( "TreeMaker created %i new scalars and %i new vectors.", scalerCount, vectorCount ) def clear(self): @@ -89,6 +96,7 @@ def clear(self): def fill(self): # Write to TTree + #self.debugBranchAddresses( prefix = "Filling") if self.treeIsExternal: for b in self.branches: b.Fill() diff --git a/core/python/TreeReader.py b/core/python/TreeReader.py index be4822b..1030a7f 100644 --- a/core/python/TreeReader.py +++ b/core/python/TreeReader.py @@ -45,8 +45,8 @@ def __init__(self, sample, variables=[], sequence = [], selectionString = None, # Whether all branches are to be read or whether that information should come from the variables self.allBranchesActive = allBranchesActive - # Read branch information from the chain - self.readLeafInfo() + ## Read branch information from the chain -> Will be useful when using auto-type + #self.readLeafInfo() # 'variables' are read from the chain super(TreeReader, self).__init__( variables = list(set(variables)) ) @@ -59,18 +59,19 @@ def __init__(self, sample, variables=[], sequence = [], selectionString = None, # set the addresses of the branches corresponding to 'variables' self.setAddresses() - + # make eList from cutString # Turn on everything for flexibility with the selectionString logger.debug("Initializing TreeReader for sample %s", self.sample.name) self.activateAllBranches() - self.eList = self.sample.getEventList(selectionString = self.selectionString) if self.selectionString is not None else None + self._eList = self.sample.getEventList(selectionString = self.selectionString) if self.selectionString is not None else None self.activateBranches() - self.nEvents = self.eList.GetN() if self.eList else self.sample.chain.GetEntries() + self.nEvents = self._eList.GetN() if self._eList else self.sample.chain.GetEntries() logger.debug("Found %i events in %s", self.nEvents, self.sample.name) # default event range of the reader self.eventRange = (0, self.nEvents) + def setAddresses(self): ''' Set all the branch addresses to the members in the class instance @@ -84,26 +85,25 @@ def setAddresses(self): self.sample.chain.SetBranchAddress(comp.name, ROOT.AddressOf(self.event, comp.name )) else: raise ValueError( "Don't know what variable %r is." % s ) - def cloneTree(self, branchList = [], newTreename = None, rootfile = None): '''Clone tree after preselection and event range ''' selectionString = self.selectionString if self.selectionString is not None else "1" - if self.eList: + if self._eList: # If there is an eList, first restrict it to the event range, then clone + import uuid + name = str(uuid.uuid4()) list_to_copy = ROOT.TEventList("tmp","tmp") for i_ev in xrange(*self.eventRange): - list_to_copy.Enter(self.eList.GetEntry(i_ev)) + list_to_copy.Enter(self._eList.GetEntry(i_ev)) + + self.sample.chain.GetEntry(list_to_copy.GetEntry(0)) #This is needed to keep branch addresses when running over a few events and >=1 file # activate branches that we want to copy, disable the ones we only need for reading self.activateBranches( turnOnReadBranches = False, branchList = branchList ) - # preserving current event list - tmpEventList = self.sample.chain.GetEventList() - tmpEventList = 0 if not self.sample.chain.GetEventList() else tmpEventList # Copy only the selected events - self.sample.chain.SetEventList( list_to_copy ) # Create the new tree in a file (if there is one) tmp_directory = ROOT.gDirectory @@ -113,42 +113,19 @@ def cloneTree(self, branchList = [], newTreename = None, rootfile = None): # Copying tree - # 1. causes trouble with few events and few files. - # res = self.sample.chain.CopyTree( "(1)", "" ) - - # 1.5: this seems to give misaligned l1_pt and LepGood_pt??? - # tree = self.sample.chain.GetTree() - # tree.SetEventList( list_to_copy ) - # res = tree.CopyTree( "(1)", "" ) - - # 2. Doesn't take into account event list - #res = self.sample.chain.CloneTree( 0 ) - #res.CopyEntries( self.sample.chain ) - - # 3. Loop (no problems observed) logger.debug("Copying %i events in a loop.", list_to_copy.GetN()) - #tree = self.sample.chain.GetTree() - res = self.sample.chain.GetTree().CloneTree( 0 ) + res = self.sample.chain.GetTree().CloneTree( 0 ) for i_event in xrange(list_to_copy.GetN()): self.sample.chain.GetEntry( list_to_copy.GetEntry(i_event) ) res.Fill() - # Needed? - res.Write() - ## 4. Doesn't take into account event list - #tree = self.sample.chain.GetTree() - #tree.SetEventList( tmpEventList ) - #res = tree.CloneTree( 0 ) - #res.CopyEntries( tree ) + res.Write() logger.debug("Number of events: list_to_copy %i res.GetEntries() %i", list_to_copy.GetN(), res.GetEntries()) # Change back to previous gDirectory tmp_directory.cd() - # restoring event list - self.sample.chain.SetEventList( tmpEventList ) - # activate what we read, don't activate the ones we just copied self.activateBranches( turnOnReadBranches = True, branchList = [] ) @@ -197,28 +174,28 @@ def activateAllBranches(self): ''' self.sample.chain.SetBranchStatus("*", 1) - def readLeafInfo(self): - ''' Read information on the leaves from the chain and store in dict - FIXME: Pretty sure this doesn't yet work for fixed sized vectors - ''' - leafInfo = [] - for s in self.sample.chain.GetListOfLeaves(): - leaf = {'type':shortTypeDict[s.GetTypeName()], 'name':s.GetName()} - countval = array.array('i',[-9999]) - pointer = s.GetLeafCounter(countval) - leaf['dim'] = 'scalar' - if pointer: - # Vector - # GetLeafCounter returns countval==1 if a counter leaf was found and pointer points to that leaf - # https://root.cern.ch/doc/master/classTLeaf.html#a062bd89a11fd1f922c096e66d2601ab6 - if countval[0]==1: - leaf['counterInt'] = pointer.GetName() - leaf['dim'] = 'vector' - else: - # For fixed size arrays, the pointer is zero and the countval is the size of the array - leaf['nMax'] = countval[0] - leafInfo.append(leaf) - return leafInfo +# def readLeafInfo(self): +# ''' Read information on the leaves from the chain and store in dict +# FIXME: Pretty sure this doesn't yet work for fixed sized vectors +# ''' +# leafInfo = [] +# for s in self.sample.chain.GetListOfLeaves(): +# leaf = {'type':shortTypeDict[s.GetTypeName()], 'name':s.GetName()} +# countval = array.array('i',[-9999]) +# pointer = s.GetLeafCounter(countval) +# leaf['dim'] = 'scalar' +# if pointer: +# # Vector +# # GetLeafCounter returns countval==1 if a counter leaf was found and pointer points to that leaf +# # https://root.cern.ch/doc/master/classTLeaf.html#a062bd89a11fd1f922c096e66d2601ab6 +# if countval[0]==1: +# leaf['counterInt'] = pointer.GetName() +# leaf['dim'] = 'vector' +# else: +# # For fixed size arrays, the pointer is zero and the countval is the size of the array +# leaf['nMax'] = countval[0] +# leafInfo.append(leaf) +# return leafInfo def getEventRanges(self, maxFileSizeMB = None, maxNEvents = None, nJobs = None, minJobs = None): '''For convinience: Define splitting of sample according to various criteria @@ -252,14 +229,14 @@ def setEventRange( self, evtRange ): self.eventRange = ( max(0, evtRange[0]), min( self.nEvents, evtRange[1]) ) logger.debug( "[setEventRange] Set eventRange %r (was: %r) for reader of sample %s", self.eventRange, old_eventRange, self.sample.name ) - def setEventList( self, evtList ): - ''' Specify an event list that the reader will run over. - ''' - self.sample.chain.SetEventList( evtList ) - self.eList = evtList - self.nEvents = self.eList.GetN() - self.eventRange = (0, self.nEvents) - logger.debug( "[setEventList] Set eventRange %r for reader of sample %s", self.eventRange, self.sample.name ) + #def setEventList( self, evtList ): + # ''' Specify an event list that the reader will run over. + # ''' + # self.sample.chain.SetEventList( evtList ) + # self.eList = evtList + # self.nEvents = self.eList.GetN() + # self.eventRange = (0, self.nEvents) + # logger.debug( "[setEventList] Set eventRange %r for reader of sample %s", self.eventRange, self.sample.name ) def reduceEventRange( self, reduction_factor ): ''' Reduce event range by a given factor. From f8727cc17fb85eea20fc202f9f91ba14bfe206a6 Mon Sep 17 00:00:00 2001 From: Robert Schoefbeck Date: Wed, 8 Mar 2017 10:34:13 +0100 Subject: [PATCH 3/4] fix bug when running over a small number of events. More care with event lists --- core/python/TreeReader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/python/TreeReader.py b/core/python/TreeReader.py index 1030a7f..07cc1a4 100644 --- a/core/python/TreeReader.py +++ b/core/python/TreeReader.py @@ -276,7 +276,7 @@ def _execute(self): # get entry errorLevel = ROOT.gErrorIgnoreLevel ROOT.gErrorIgnoreLevel = 3000 - self.sample.chain.GetEntry ( self.eList.GetEntry( self.position ) ) if self.eList else self.sample.chain.GetEntry( self.position ) + self.sample.chain.GetEntry ( self._eList.GetEntry( self.position ) ) if self._eList else self.sample.chain.GetEntry( self.position ) ROOT.gErrorIgnoreLevel = errorLevel # sequence From 59a5cd40339c5f07d3f26cd0d665326426785aea Mon Sep 17 00:00:00 2001 From: UAEDF-tomc Date: Wed, 8 Mar 2017 16:17:10 +0100 Subject: [PATCH 4/4] could have more columns in the legend (backwards compatible) --- plot/python/plotting.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/plot/python/plotting.py b/plot/python/plotting.py index ef9885d..d9b30f5 100644 --- a/plot/python/plotting.py +++ b/plot/python/plotting.py @@ -341,6 +341,12 @@ def draw(plot, \ max_ = max( l[0].GetMaximum() for l in histos ) min_ = min( l[0].GetMinimum() for l in histos ) + # If legend is in the form (tuple, int) then the number of columns is provided + legendColumns = 1 + if len(legend) == 2: + legendColumns = legend[1] + legend = legend[0] + #Calculate legend coordinates in gPad coordinates if legend is not None: if legend=="auto": @@ -424,6 +430,7 @@ def draw(plot, \ # Make the legend if legend is not None: legend_ = ROOT.TLegend(*legendCoordinates) + legend_.SetNColumns(legendColumns) legend_.SetFillStyle(0) # legend_.SetFillColor(0) legend_.SetShadowColor(ROOT.kWhite)