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

Iss752: Add driver for overlaying signal onto beam events #756

Merged
merged 11 commits into from
Feb 5, 2021

Conversation

JeremyMcCormick
Copy link
Member

@JeremyMcCormick JeremyMcCormick commented Nov 28, 2020

This new driver can be used externally to generate a file with combined beam and signal events, replacing usage of the FilterMCBunches and LCIO merge tools. This has several advantages, including the elimination of several steps in the job chain as well as no longer writing a large "spaced" file with the signal events before they are merged. This should reduce disk usage of MC jobs, as well as improve the overall runtime performance.

Signal events can be optionally rejected as they are read using filters on total ECAL edep and number of Hodoscope hits (more filters can easily be added, as needed, by implementing the EventFilter interface and adding the appropriate setter methods).

I have provided an example steering file to run only the overlay and produce a combined file, as well as a sample steering file for running the overlay inline with the readout.

I verified that the new scheme produces triggers in the readout simulation by using examples in hps-mc to generate the beam and signal (tritrig) files and then running them through the sample readout configuration.

@JeremyMcCormick JeremyMcCormick linked an issue Nov 28, 2020 that may be closed by this pull request
@JeremyMcCormick JeremyMcCormick added this to the V4.5 milestone Nov 28, 2020
@mholtrop
Copy link
Collaborator

Are there any studies that compare this with the previous method?
It seems to me that before we change the way we do MC, we need to make sure this does not change the results.

@JeremyMcCormick
Copy link
Member Author

JeremyMcCormick commented Jan 12, 2021 via email

@cbravo135
Copy link
Collaborator

Changes would have to be made to hps-mc to actually use any of this.

@JeremyMcCormick
Copy link
Member Author

JeremyMcCormick commented Jan 26, 2021 via email

@tongtongcao
Copy link
Contributor

tongtongcao commented Jan 26, 2021

Jeremy and I ever had a deep discussion about this driver.
The driver can replace filter tools in hps-java and merging tool in LCIO, and combine two steps (filtering and merging) together.
I ever did a basic test by 2016 MC. With the same setup for filtering, # of triggers from readout by the old processing and by updated processing with application of this driver is reasonably close.

Generally, I would say that this driver works well. It can save space and time comparing to the old MC processing.
For normalization of MC data, it requires to run out of signal events with enough beam events for merging.
If we run out of signal events before running through all the requested beam events, an exception is thrown from the driver, but the output file would still be written out. The exception is not easy to be avoided, but it does not affect MC processing.
Additionally, the driver does not work if we need pure signal samples.

Certainly, with application of the driver, readout steering files and hps-mc job scripts need to be updated.
Before these updates, we need to determine how to fix the issue for production of pure signal samples. Still using the old processing? Applying a file with all empty events as beam for merging, or other ways? Choice of different ways affects how we do updates for readout steering files and hps-mc job scripts.

@tongtongcao
Copy link
Contributor

The driver works. I prefer to approve this PR, and to leave corresponding updates for readout steering files later.

@JeremyMcCormick JeremyMcCormick removed the request for review from tongtongcao February 5, 2021 01:18
Copy link
Collaborator

@cbravo135 cbravo135 left a comment

Choose a reason for hiding this comment

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

I agree with TongTong

@cbravo135 cbravo135 merged commit 82b5255 into master Feb 5, 2021
@JeremyMcCormick JeremyMcCormick modified the milestones: V4.5, v5.0 Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add driver for overlaying signal onto beam events
4 participants