-
Notifications
You must be signed in to change notification settings - Fork 83
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
Add RMC as a muon stop pileup product option #1375
base: main
Are you sure you want to change the base?
Conversation
Hi @michaelmackenzie,
which require these tests: build. @Mu2e/fnalbuild-users, @Mu2e/write have access to CI actions on main. ⌛ The following tests have been triggered for e944805: build (Build queue is empty) |
☀️ The build tests passed at e944805.
N.B. These results were obtained from a build of this Pull Request at e944805 after being merged into the base branch at f5aa30e. For more information, please check the job page 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.
I just realized that this tool is using unvalidated fhicl. We have the policy that all (legacy) codes using unvalidated fhicl must upgrade when they are modified. Validated fhicl examples can be found throughout our code base, look for ‘Config’ structs.
@@ -125,6 +125,7 @@ double physicsParams.Al.capture.protonRate = 0.05; | |||
double physicsParams.Al.capture.deuteronRate = 0.025; | |||
double physicsParams.Al.capture.neutronRate = 1.2; | |||
double physicsParams.Al.capture.photonRate = 2.0; | |||
double physicsParams.Al.capture.RMCRate = 9.460e-5; |
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.
Is there provenance for this number? Does it implicitly assume a cutoff energy? Can that be documente?
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.
This is from the TRIUMF measurement of 1.4e-5 for E > 57 MeV, divided by the closure fraction above 57 MeV. Should I add this as a comment here or document it somewhere else?
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.
Please add it as a comment after this line. We need to document the provenance of every physics number in this table, which is beyond the scope of this PR. I will open an issue once the PR is merged
@@ -45,14 +45,16 @@ namespace mu2e { | |||
_czmin(conf().spectrum.get<fhicl::ParameterSet>().get<double>("czmin", -1.)), | |||
_czmax(conf().spectrum.get<fhicl::ParameterSet>().get<double>("czmax", 1.)), | |||
_spectrum(BinnedSpectrum(conf().spectrum.get<fhicl::ParameterSet>())), | |||
_useRate(conf().spectrum.get<fhicl::ParameterSet>().get<bool>("useRate", false)), | |||
_internalRate((_external) ? 0. : 1.), |
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.
I’m struggling to follow the settings needed to get the following configurations
- external (photons) only
- internal conversions only
- physical admixture of internal and external
Some combination of the ‘external’ and ‘userate’ bools seems to provide these, but I can’t quite follow.
It would be clearer if the valid configuration options were explicitly enumerated (enum) and set in the config.
I also find the term ‘external’ for producing RMC photons unintuitive, as those photons might be detected directly in the calorimeter, in which case there’s nothing ‘external’ about them.
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.
Only one of the external
or useRate
flags are necessary. I updated the configuration to use validated fhicl and in the update I set the external
flag to only be required if useRate
is false. I could update this to use strings or enums to check for something like "natural", "external", or "internal" if this is a preferred logic. I also don't prefer external
as I term (I typically write real photons or virtual photons/internal conversions in talks), but it better matches the process IDs used and alternate generators.
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.
I prefer the conceptually more clear config to choose between the 3 sensible options. The names you give above are clear. If the process names are unclear we can change them, but lets do that in a separate PR.
Add logic for RMC to be included as a muon stop pileup product. This mostly entailed adding a flag to the RMC tool to use the RMC rate per muon to generate a random number of photons (usually 0), and add the corresponding global constant. The global constant RMC rate for aluminum is 1.4e-5 for E > 57 MeV over the closure approximation spectrum fraction for E > 57 MeV (~1/10,000 muon captures). I will make a corresponding PR in Production adding this product to the MuStopPileup configuration.