-
Notifications
You must be signed in to change notification settings - Fork 54
Feature/acastill realistic pmt mc #876
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
Conversation
This reverts commit 05846ef.
|
Yeah agreed, I'd like to wait to change the input label to avoid any conflicts, and we can put in a patch for final version. I don't think the pulse oscillation will have a meaningful impact on the validation II sample (for the software trigger) since we'll primarily be concerned with agreement for low energies, not high. |
| void AddOscillationAfterPulse( std::vector<double>& wave); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The oscillation is being handled by a different module right? I think this function is unused.
| void AddOscillationAfterPulse( std::vector<double>& wave); |
| fhicl::ParameterSet GainFluctuationsParams; | ||
| bool SimulateNonLinearity; //Fluctuate PMT gain | ||
| bool PositivePolarity; | ||
| bool OscillateAfterPulse; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also unused
| bool OscillateAfterPulse; |
| bool MakeGainFluctuations; //Fluctuate PMT gain | ||
| fhicl::ParameterSet GainFluctuationsParams; | ||
| bool SimulateNonLinearity; //Fluctuate PMT gain | ||
| bool PositivePolarity; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused? The waveform polarity is directly inferred from the SER in this line
| bool PositivePolarity; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gazing at SBNDDecoSimpleFlashTPC0_data and SBNDDecoSimpleFlashTPC0_realisticMC, the parameters look completely identical.
Can we define a common table from which both _data and _realisticMC inherit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh, I just saw you did it for the OpHit configuration, can we follow the same implementation here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, just define a common table to avoid duplication and possible mis-configurations in the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add units for the configuration parameters (frequencies and OscillationThreshold) for easier future reference?
|
trigger build LArSoft/lar*@LARSOFT_SUITE_v10_14_00 SBNSoftware/sbndaq-artdaq-core@v1_10_06 SBNSoftware/sbn*@SBN_SUITE_v10_14_00 |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for e26:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
trigger build LArSoft/lar*@LARSOFT_SUITE_v10_14_00 SBNSoftware/sbndaq-artdaq-core@v1_10_06 SBNSoftware/sbn*@SBN_SUITE_v10_14_00 |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for SBND Failed at phase ci_tests SBND on slf7 for e26:prof - ignored failure for unit_test - ignored warnings for build -- details available through the CI dashboard 🚨 For more details about the failed phase, check the ci_tests SBND phase logs parent CI build details are available through the CI dashboard |
Description
This PR aims at providing a realistic PMT MC simulation at the detsim stage. In particular it modifies the following items:
PMTGaussianGainFluctuation_tool.ccthat allows simulating a different gaussian gain fluctuation for each PMT.PMTNonLinearityTF1ChannelByChannel_tool.ccthat allows simulating pmt non-linearity on a channel by channel basis.PMTPulseOscillation_module.ccthat reproduces the low-frequency oscillations that have been observed after large-amplitude signals. More information here ().This PR does all the mentioned changes to the standard simulation workflow. It does also refactor the PMT-related fcl configuration files so there is only one single configuration file that contains ideal MC, realistic MC and data configurations.
Checklist
Reviewers,AssigneesDevelopementRelevant PR links (optional)
Does this PR require merging another PR in a different repository (such as sbnanobj/sbnobj etc.)?
This PR requires merging
#863
SBNSoftware/sbnd_data#4
Link(s) to docdb describing changes (optional)
Most of the changes are described here https://sbn-docdb.fnal.gov/cgi-bin/sso/ShowDocument?docid=43468