Skip to content
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

Generalisation of Interpolation modules #35

Open
wants to merge 5 commits into
base: CMSSW_10_X
Choose a base branch
from

Conversation

tjavaid
Copy link
Collaborator

@tjavaid tjavaid commented Apr 22, 2022

Minor update in the interpolation macro to incorporate the more Higgs mass points and the production modes.

Need to edit the lines to select the mass points and production modes (execution command remains same as per instructions in CMSSW_10_X branch)

@ram1123
Copy link
Collaborator

ram1123 commented Apr 22, 2022

Hi @tjavaid ,

Please edit the PR target branch of the PR. Currently you are pushing it to the master. You should push it to CMSSW_10_X

@tjavaid tjavaid changed the base branch from master to CMSSW_10_X April 22, 2022 11:54
@ram1123
Copy link
Collaborator

ram1123 commented May 4, 2022

Hi @tjavaid ,

This PR has some conflicts. Please resolve this. Then everything should be fine.

@tjavaid
Copy link
Collaborator Author

tjavaid commented May 4, 2022

resolved. But I see a tab space is added once I resolved. You can see here:
https://github.com/vukasinmilosevic/Fiducial_XS/blob/CMSSW_10_X_interpolatePlus/python/interpolate_differential_pred.py#L165

Should I do git pull at the working area and then commit again after removing this ?

@ram1123
Copy link
Collaborator

ram1123 commented May 4, 2022

resolved. But I see a tab space is added once I resolved. You can see here: https://github.com/vukasinmilosevic/Fiducial_XS/blob/CMSSW_10_X_interpolatePlus/python/interpolate_differential_pred.py#L165

Should I do git pull at the working area and then commit again after removing this ?

I think this will break the code. Did you check by running these modules?

Please confirm the following two points:

  1. Is the updated module working fine?
  2. Is the dictionaries created by new module and the old one are identical?

Once I get your response with "YES" for both question, I will check them.

@tjavaid
Copy link
Collaborator Author

tjavaid commented May 4, 2022

yes, PR was created after checking by running the modules (for mass4l and pT4l) and was working fine.
Yes, the dictionary names are identical (had to rename to generalize for other production modes and treatment to MG5 sample was done which was not there before) as you can check here:

616eedb

@ram1123
Copy link
Collaborator

ram1123 commented May 4, 2022

yes, PR was created after checking by running the modules (for mass4l and pT4l) and was working fine.

Okay. By eye it seems that there is an indentation issue. Since you said its running then it should be fine.

Yes, the dictionary names are identical (had to rename to generalize for other production modes) as you can check here:

616eedb

I am asking about the values in the output dictionary. Are they also same?

@ram1123
Copy link
Collaborator

ram1123 commented May 4, 2022

Hi @tjavaid ,

Seems like you didn't update your latest macro. As your macro on the GitHub breaks.

Command: python python/interpolate_differential_pred.py --obsName="pT4l" --obsBins="|0|10|20|30|45|60|80|120|200|13000|" --year=2018
  File "python/interpolate_differential_pred.py", line 166
    f.write('acc = '+str(acc_all)+' \n')
                                       ^
IndentationError: unindent does not match any outer indentation level

@tjavaid
Copy link
Collaborator Author

tjavaid commented May 4, 2022

yes, PR was created after checking by running the modules (for mass4l and pT4l) and was working fine.

Okay. By eye it seems that there is an indentation issue. Since you said its running then it should be fine.

Yes, the dictionary names are identical (had to rename to generalize for other production modes) as you can check here:
616eedb

I am asking about the values in the output dictionary. Are they also same?

Yes, I checked that the values in the dictionaries I got are same for ggH and NNLOPS with both versions of the macro.

The, additional tab space appeared when I resolved conflicts. Before resolving the conflicts it was not there as you can see here:

616eedb

@ram1123
Copy link
Collaborator

ram1123 commented May 4, 2022

Hi @tjavaid ,

Since, its not working it means there was an issue while you were trying to resolve the conflict.

Please check the two points again (after resolving the conflict)

  1. Is the updated module working fine?
  2. Is the dictionaries created by new module and the old one are identical?

And these two points are general. Whenever you resolve conflict you have to be careful. And re-check the program by running and checking the results.

@ram1123 ram1123 added the enhancement New feature or request label May 4, 2022
@ram1123 ram1123 added this to the First running version of 2018 milestone May 4, 2022
@ram1123 ram1123 changed the title Cmssw 10 x interpolate plus Generalisation of Interpolation modules May 4, 2022
@vukasinmilosevic
Copy link
Owner

@tjavaid any news on this?

@tjavaid
Copy link
Collaborator Author

tjavaid commented May 9, 2022

@vukasinmilosevic, done with this. Please check.

@vukasinmilosevic
Copy link
Owner

Hi @ram1123 @tjavaid, this one is ready to go in?

@vukasinmilosevic
Copy link
Owner

@ram1123, @qyguo, your previous PR already include this in some form, right? Can I close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants