Convert boost hists to ROOT TGraphs #17
Conversation
dsavoiu
left a comment
dsavoiu
left a comment
There was a problem hiding this comment.
Thanks for the PR, @apaasch. It looks OK to me so far, I only have a few comments related to code style and variable naming, but this should be easy to fix. One should also maybe clarify if the convert_to_root.py script is meant to run while columnflow is sourced or completely outside (it would also be good to add this information to the file).
dijet/tasks/base.py
Outdated
| raise RuntimeError( | ||
| "no single dataset found in config matching " | ||
| f"process `{self.branch_data.process}`" | ||
| "no single dataset found in config matching ", |
There was a problem hiding this comment.
No comma needed here since the string continues on the next line.
| "no single dataset found in config matching ", | |
| "no single dataset found in config matching " |
dijet/tasks/base.py
Outdated
| raise RuntimeError( | ||
| "no single user-supplied dataset matched " | ||
| f"process `{self.branch_data.process}`" | ||
| "no single user-supplied dataset matched ", |
There was a problem hiding this comment.
| "no single user-supplied dataset matched ", | |
| "no single user-supplied dataset matched " |
dijet/scripts/convert_to_root.py
Outdated
| print(f"{arg}: {getattr(args, arg)}") | ||
|
|
||
| # Construct the file path | ||
| base_path = "/nfs/dust/cms/user/paaschal/WorkingArea/Analysis/JERC/DiJet/data/dijet_store/analysis_dijet/" |
There was a problem hiding this comment.
Can this script be run inside a columnflow environment (i.e. after sourcing setup.sh)? If so, it would be good to avoid hardcoding the path here and use the environment variables insteda (e.g. os.getenv("CF_STORE_LOCAL")).
There was a problem hiding this comment.
Yes it can be run within the CF environment. You suggestion will be added to the next commit.
Redo flake8 changes Change naming scheme and structur in root task to be more efficient and readable. Co-authored-by: Daniel Savoiu <[email protected]>
apaasch
left a comment
apaasch
left a comment
There was a problem hiding this comment.
Thanks, all your suggestion have been added.
dijet/scripts/convert_to_root.py
Outdated
| print(f"{arg}: {getattr(args, arg)}") | ||
|
|
||
| # Construct the file path | ||
| base_path = "/nfs/dust/cms/user/paaschal/WorkingArea/Analysis/JERC/DiJet/data/dijet_store/analysis_dijet/" |
There was a problem hiding this comment.
Yes it can be run within the CF environment. You suggestion will be added to the next commit.
dijet/tasks/base.py
Outdated
| raise RuntimeError( | ||
| "no single dataset found in config matching " | ||
| f"process `{self.branch_data.process}`" | ||
| "no single dataset found in config matching ", |
dijet/tasks/base.py
Outdated
| raise RuntimeError( | ||
| "no single user-supplied dataset matched " | ||
| f"process `{self.branch_data.process}`" | ||
| "no single user-supplied dataset matched ", |
dsavoiu
left a comment
dsavoiu
left a comment
There was a problem hiding this comment.
Thanks for the changes! Looks good to me, but there is one small change I would make to the convert_to_root.py script (see below).
dsavoiu
left a comment
dsavoiu
left a comment
There was a problem hiding this comment.
Thanks, this looks good to me now. Let's merge!
2 participants
Task structure to store boost histogram objects as TGraphAsymmErrors in rootfiles.
Main goal is to do this indepently from Root using uproot.
This is not yet possible but there is ongoing work from the uproot team addressing this issue.
Updates can be followed in this PR from uproot5.
As long as this feature is not yet implement the conversion is done in two steps:
In the long term, the second step is included in the first one and remove the dependency from Root.
Additional