-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2D plotting changes #15
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I just have one comment.
UPDATE: In commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry this went unnoticed for so long @abasnet97. I think all the changes look good. If you can confirm that this still runs without issue, feel free to merge it.
@bryates, I have checked that things still work fine with this branch. I was able to run I have also merged changes from master and cleaned the conflicts. Let me know if you would like me to do some more checks before we can get this merged to master. |
rootFile = ROOT.TFile.Open('../fit_files/higgsCombine{}.MultiDimFit.root'.format(name)) | ||
limitTree = rootFile.Get('limit') | ||
rootFile = self.CreateNewLimitTreefor2DScan(name_lst,wcs) | ||
limitTree = rootFile.limit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line actually works? I would have expected it to be
limitTree = rootFile.Get('limit')
@@ -185,6 +191,28 @@ def GetWCsNLLFromRoot(self,base_name_lst,wc,unique=False): | |||
return [graphwcs,graphnlls] | |||
|
|||
|
|||
def CreateNewLimitTreefor2DScan(self,name_lst,wc_lst): | |||
graphwcs, graphnlls = self.GetWCsNLLFromRoot(name_lst,wc_lst,unique=True) | |||
newRootFile = ROOT.TFile("tmp.root","RECREATE") #temporary root file with new limit tree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be more efficient to pass around the tree instead of making a temporary file. Don't worry about doing that in this PR, but maybe just something to think about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea. I think my logic back then was that this function would allow us to dump out a root file (if need be), and we could investigate the TTree inside this file if we wanted to. Since this was 2 years ago, I definitely didn't have as good of an understanding of the code as I have now, so I would probably have opted for a different way.
This PR pertains to the changes made to the 2D plotting methods in EFTPlotter script to handle duplicate point removal as a result of implementation of random starting points for 2D scans in MultiDimFit method. There are two main changes here:
Both the changes have been tested in multiple cases, and they work as expected.