-
Notifications
You must be signed in to change notification settings - Fork 9
adding R2_R3_cent_comparison.py #15
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: dev
Are you sure you want to change the base?
Conversation
|
Hi @F-Ali-Khan ! Thanks for the development. I have few comments that I leave here below. Let me know if you need help with those |
hi @stefanopolitano I cannot see the comments, you posted. |
|
Hi @stefanopolitano Please see the code i have modify it according to your comments ...let me know if there is needed any further changing |
stefanopolitano
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.
Hi @F-Ali-Khan ! Thanks for the update, I still have a couple of points you might want to address
src/compute_Resolution.py
Outdated
| sys.path.append(utils_path) | ||
|
|
||
| # Import your custom functions | ||
| from my_utils import get_resolution, get_centrality_bins, getListOfHisots |
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.
What is this "my_utils" folder? Could you align it with the current folder structure? Also, make sure the utils you are considering here are also present in the repository
src/compute_Resolution.py
Outdated
| histos_triplets, histos_triplets_lables = getListOfHisots(an_res_file, wagon_id, vn_method) | ||
|
|
||
| # prepare output file | ||
| if vn_method == 'sp': |
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 comment this here, but it counts for all the instances. We do not use the ep or deltaphi methods anymore. It would be cleaner to remove the vn_method option all over from the code
| sys.path.append(utils_path) | ||
|
|
||
| # Import your custom functions from utils | ||
| from utils import get_resolution, get_centrality_bins, getListOfHisots |
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.
Hi @F-Ali-Khan ! Thanks for the update. We should include also these utility functions in the utils of the repository, otherwise the script will break
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.
Hi @stefanopolitano (get_resolution, get_centrality_bins, getListOfHisots) these all functions is define in the utills,py and i run this script it is working properly ...
|
Hi @F-Ali-Khan ! Any updates on this? :) |
No description provided.