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

Adding FCC-hh analysis example, code and event weight processing #411

Merged
merged 31 commits into from
Oct 16, 2024

Conversation

bistapf
Copy link
Contributor

@bistapf bistapf commented Oct 14, 2024

This PR adds the FCC-hh analysis code in a dedicated analyzer, as well as a first small working FCC-hh example (removing all the no longer working old ones), as well as code for processing the event weights (which are != 1 in FCC-hh samples) correctly. All for edm4hep v1, but not yet using DataSource.

Things to note:

  • Processing of the event weights is done only if the optional argument do_weighted is set from the respective analysis python script. Behaviour for analyses not setting this should be unchanged. Validity of the normalisation with sum of weights and cross-section has been checked with a small sample of bbyy events.
  • The code to retrieve heavy flavour tagging from Delphes is technically working, but does not produce expected physics results in tests. This is to be fixed still. -> PR131 in k4SimDelphes fixes this, the b-tagging efficiency has been confirmed to be as expected for the FCC-hh Delphes card using a small sample of bbyy events.

python/run_fccanalysis.py Outdated Show resolved Hide resolved
python/run_fccanalysis.py Outdated Show resolved Hide resolved
python/run_fccanalysis.py Outdated Show resolved Hide resolved
python/run_fccanalysis.py Outdated Show resolved Hide resolved
python/run_fccanalysis.py Outdated Show resolved Hide resolved
if do_weighted:
LOGGER.info('Using generator weights')
sow_process={}
sow_ttree={}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use process_events?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you meant to fully replace it (not just init it as the same), I think we also want to keep the raw number of events to check the MC statistics e.g., don't we?

@kjvbrt
Copy link
Contributor

kjvbrt commented Oct 15, 2024

Is there any reason, why not to introduce default weight of the event to be equal to one regardless whether there are input files which contains the information or not?

@kjvbrt
Copy link
Contributor

kjvbrt commented Oct 15, 2024

Can you also add an explanation for the do_weights analysis attribute to the manual pages?
https://github.com/HEP-FCC/FCCAnalyses/blob/master/man/man7/fccanalysis-script.7

@bistapf
Copy link
Contributor Author

bistapf commented Oct 15, 2024

Is there any reason, why not to introduce default weight of the event to be equal to one regardless whether there are input files which contains the information or not?

Do you mean instead of having the do_weighted option? Just add a weight column of 1s if the file doesn't have it and always to SoW calculation etc.? I guess there is no real major reason, other than then you carry around superfluous information in case there is no weight right? E.g. the SoW parameter doesn't need to be written if the weights are 1 right? So this was just my idea to keep it behind an option, but happy to change if you think it's better as standard behaviour.

@kjvbrt
Copy link
Contributor

kjvbrt commented Oct 15, 2024

Yes, to have 1s as default. Having one more column is I think reasonable trade-off. There are some analysis on the FCCee side coming which will need event weights as well.

@bistapf
Copy link
Contributor Author

bistapf commented Oct 15, 2024

Yes, to have 1s as default. Having one more column is I think reasonable trade-off. There are some analysis on the FCCee side coming which will need event weights as well.

Ok, sure. But could we just merge it like this with the option for now, and then I make the changes to not use the option anymore but default weight = 1 in a separate PR?

@bistapf
Copy link
Contributor Author

bistapf commented Oct 16, 2024

Can you also add an explanation for the do_weights analysis attribute to the manual pages? https://github.com/HEP-FCC/FCCAnalyses/blob/master/man/man7/fccanalysis-script.7

Done.

@kjvbrt kjvbrt merged commit 5d6d997 into HEP-FCC:master Oct 16, 2024
5 checks passed
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.

2 participants