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

code used to produce 2023C,D SF #81

Merged
merged 75 commits into from
Sep 17, 2024
Merged

Conversation

saswatinandan
Copy link
Contributor

No description provided.

“saswatinandan” and others added 4 commits August 23, 2024 16:12
@@ -117,3 +117,5 @@ To create ROOT files including the measured SFs please install [`TauIDSFs` tool]
Modify the `TauIDSFs/utils/createSFFiles.py` script to include your measured SFs into the script.
Finally, run the `TauFW/scripts/tau_createROOT.sh` to generate your ROOT files. They will be created into `TauFW/scripts/data/`
IMPORTANT: please comment and do not delete older SFs

The latest code used to create root files for SF can be taken from [here](https://github.com/saswatinandan/TauFW/blob/master/Fitter/createroot_TES.py)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this link be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used this code to produce the root files containing all the sfs. If there is any other code to produce this, it can be linked here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this should be the README to enrich/update with the latest version of scripts running command.

@IzaakWN
Copy link
Collaborator

IzaakWN commented Aug 26, 2024

Hi Saswati, thank you for the quick follow-ups! Barring some minor comments above and maybe pending review from @pmastrap, I think this PR is almost ready to be merged.

@@ -90,6 +105,7 @@ def harvest(setup, year, obs, **kwargs):
scaleFactor = 1.0
if "scaleFactor" in sysDef:
scaleFactor = sysDef["scaleFactor"]
if "name" in sysDef: sysDef["processes"] = check_integral(filename, sysDef["processes"], sysDef["name"], region)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's this new function for ? can you put a comment line to explain, pls?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some systematic, I saw that statistics was too small, actually zero, and in some cases it was giving nan value. If those samples were removed from the samples list, nan value was removed.

Copy link
Collaborator

@pmastrap pmastrap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @saswatinandan @IzaakWN, just done with the review.
I think that the PR is in a good shape with the exception of few small changes, needed before merging.
Additionally, I would suggest the update of the TauES_ID README with prompt instructions to run the full workflow, to make it easier for future measurements.

@saswatinandan
Copy link
Contributor Author

Hi,

I tried to implement all the comments as much as possible, if still there is some problem, please let me know.

@pmastrap
Copy link
Collaborator

pmastrap commented Sep 5, 2024

Thanks @saswatinandan for all the work! I think the only thing missing is the control on the era in ModuleMuMu.
Everything else look fine :)

@pmastrap
Copy link
Collaborator

pmastrap commented Sep 6, 2024

I think this PR is ready to be merged @IzaakWN

@pmastrap pmastrap merged commit 51ce1e1 into cms-tau-pog:master Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants