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

Improvement of the HZZ Analysis notebook #49

Merged
merged 6 commits into from
Sep 9, 2024

Conversation

hexagonsexagon
Copy link
Collaborator

@hexagonsexagon hexagonsexagon commented Aug 1, 2024

I have restructured the notebook such that the code is more readable and understandable to someone who is new to Python. The notebook has been split up into 3 parts: example 1: Reading data, example 2: Reading Monte-Carlo data, and Final Analysis. The first two examples go over the data analysis for one sample of the data and the Monte-Carlo data respectively, while explaining how the selection rules and weights work. The final portion is the full analysis over the entire data set. I have also included a bonus section on including transverse momentum cuts. The signal significance is compared before and after the momentum cuts.

I added pedagogical explanations of all relevant physics concepts and also explanations of how the code works throughout the process of the analysis. This includes Feynman diagrams of the important production and decay modes of the Higgs as well as the conceptual basis of the signal analysis. There is also a section on frequently asked questions to answer any remaining queries the viewer may have.

Closes Issue #50

@hexagonsexagon hexagonsexagon requested a review from a team as a code owner August 1, 2024 16:02
@marianaiv marianaiv added documentation Improvements or additions to documentation enhancement New feature or request education release labels Aug 1, 2024
Copy link
Collaborator

@Soap2G Soap2G left a comment

Choose a reason for hiding this comment

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

Hi @hexagonsexagon and thanks for the PR and great job! I'm experiencing a bit of an issue in loading the diff from your updated notebook (20,160 additions, 387 deletions not shown because the diff is too large. Please use a local Git client to view these changes.), but based on this I can provide some preliminary comments. I'm gonna report them below since I'm not managing to have the normal interface for reviews.

  • L7: Why is the title now in H2? We could leave it H1.
  • L21: https
  • L70-71: I think that the images are not yet in the repo? I can't see them properly rendered.
  • L94 and following: I'm not sure why we have all these \ns here.. No new line is reported in the notebook, at least not in what I can see here.
  • L182: Same comment as for Valentina: if people are not using Binder, then this cell is needed. Could we uncomment the relevan lines?

Last question: given that the PR is requested from a fork, I guess that we are not using the hexagonsexagon-patch-1 branch, correct?

Copy link
Collaborator

@marianaiv marianaiv left a comment

Choose a reason for hiding this comment

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

Thanks for all the work Hexiang! Just a minor comment.
I couldn't load the differences. Aside from Giovanni's comments, on the significance cell it is printing:

N_sig = np.float64(30.619236946105957)
N_bg = np.float64(21.52894878387451)
signal_significance = np.float64(2.416305424196965)

Can you fix it so that it just prints the rounded number (2 or 3 decimals), without np.float.

@hexagonsexagon
Copy link
Collaborator Author

Hi @hexagonsexagon and thanks for the PR and great job! I'm experiencing a bit of an issue in loading the diff from your updated notebook (20,160 additions, 387 deletions not shown because the diff is too large. Please use a local Git client to view these changes.), but based on this I can provide some preliminary comments. I'm gonna report them below since I'm not managing to have the normal interface for reviews.

  • L7: Why is the title now in H2? We could leave it H1.
  • L21: https
  • L70-71: I think that the images are not yet in the repo? I can't see them properly rendered.
  • L94 and following: I'm not sure why we have all these \ns here.. No new line is reported in the notebook, at least not in what I can see here.
  • L182: Same comment as for Valentina: if people are not using Binder, then this cell is needed. Could we uncomment the relevan lines?

Last question: given that the PR is requested from a fork, I guess that we are not using the hexagonsexagon-patch-1 branch, correct?

  1. Fixed
  2. Fixed
  3. They will appear once Valentina's pull request is finalised.
  4. I use a writing convention in markdown and latex... I break apart new sentences and commas with new lines to make them easier to read. I can remove them if you want.
  5. Fixed

@hexagonsexagon
Copy link
Collaborator Author

Thanks for all the work Hexiang! Just a minor comment. I couldn't load the differences. Aside from Giovanni's comments, on the significance cell it is printing:

N_sig = np.float64(30.619236946105957)
N_bg = np.float64(21.52894878387451)
signal_significance = np.float64(2.416305424196965)

Can you fix it so that it just prints the rounded number (2 or 3 decimals), without np.float.

Fixed

Soap2G
Soap2G previously approved these changes Aug 7, 2024
Copy link
Collaborator

@Soap2G Soap2G left a comment

Choose a reason for hiding this comment

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

Hi @hexagonsexagon, looks good to me. Thank you.

Copy link
Collaborator

@marianaiv marianaiv left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks

@AntonioJC
Copy link
Collaborator

Hi @hexagonsexagon, thanks a lot for this work! I greatly appreciate the improved documentation and the step-by-step walkthrough of the analysis! Thanks also for addressing the comments above, I don't have any major one to raise, so I'll merge this PR. Thanks again for this project!

@AntonioJC AntonioJC merged commit 905b48f into atlas-outreach-data-tools:master Sep 9, 2024
9 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation education release enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants