-
Notifications
You must be signed in to change notification settings - Fork 4
Adapting EFTFitter to netapp #29
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
base: py3Combine
Are you sure you want to change the base?
Conversation
Fix selectedWCs path in text2workspace
…-aac-model Fix selectedWCs path in text2workspace
bryates
left a comment
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.
Thanks @anpicci. I left a few minor comments.
| print("Loading operators from {fpath}".format(fpath=fpath)) | ||
| jsn = open(fpath,'r').read() | ||
| operators = json.loads(jsn) | ||
| self.alloperators = [] |
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.
Won't removing this case errors? Since self.alloperators is set below in a if block, it could be skipped. Plus, self.alloperators.extend(operators[sig]) is called there, but we're not initializing self.alloperators as a list.
| # if level=='err': logging.error(line.rstrip('\n')) | ||
|
|
||
| def __override_CMSSW_BASE(self): | ||
| """If CMSSW_BASE points to an /afs/ path but cwd is under /users/, |
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 is a nice function, but really people should be moving their CMSSW builds from /afs/. to /users/ and run scram b ProjectRename to fix the dependencies.
|
|
||
| print(f"CMSSW_BASE after manipulation is {CMSSW_BASE}") | ||
|
|
||
| if not (workspace.startswith("/afs/") or workspace.startswith("/users/") or workspace.startswith("/scratch365/")): |
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.
Should we just check for a leading / instead. I know this covers the basis on glados and lxplus, but if a file system doesn't use one of these three paths it could still cause issues.
| ``` | ||
| text2workspace.py combinedcard.txt -o yourworkspacename.root -P EFTFit.Fitter.AnomalousCouplingEFTNegative:analiticAnomalousCouplingEFTNegative --X-allow-no-background --for-fits --no-wrappers --X-pack-asympows --optimize-simpdf-constraints=cms --PO selectedWCs=/path/to/your/selectedWCs.txt | ||
| text2workspace.py combinedcard.txt -o yourworkspacename.root -P EFTFit.Fitter.AnomalousCouplingEFTNegative:analyticAnomalousCouplingEFTNegative --X-allow-no-background --for-fits --no-wrappers --X-pack-asympows --optimize-simpdf-constraints=cms --PO selectedWCs=/path/to/your/selectedWCs.txt |
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.
Should we update the README with IM?
Here are some changes to adapt
EFTFitter.pyto netapp