-
Notifications
You must be signed in to change notification settings - Fork 1
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
Update yields and clustering #245
Conversation
for more information, see https://pre-commit.ci
Pull Request Test Coverage Report for Build 9775201621Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9778929894Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9778929894Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9790254235Details
💛 - Coveralls |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Pull Request Test Coverage Report for Build 10769196522Details
💛 - Coveralls |
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.
Not much to add, the code is very clean. I will try to test is and if it runs fine I will approve.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
* add classification via secondaries * remove prints * make more gammas... * v24 * v25 * v26 * v27 * v28 * v28 * v29 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * remove njit * cleanup * change logic for alpha and resolve comments pr * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * remove unnecessary beta classification * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * dietrofront conversion electrons * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Ciao @cfuselli, when you find some time, could you update the topmost description of your PR to account for the recent changes and the merged parallel branch? Thanks for the good work! |
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.
Very nice PR, thank you @cfuselli and @michaweiss89! I would like to wait for @ramirezdiego to have a last look and then we can merge
Refactor NestYields
The plugin is refactored to be more flexible and easier to replace single functions in alternative plugins like BetaYields.
Now we define a NestCalc object in the setup to use it in different functions. We vectorise get_quanta instead of get_quanta_from_nest and we separate yields and quanta calculations into two different functions.
The content of the plugin itself and the results are unchanged.
BetaYield modifications
The plugin now intervenes only on the results of the yields from nest, but still makes use of the getQuanta function from NEST where all the statistical fluctuations ( fano factors etc ) and recombination fluctuations are calculated.
New option: tune NEST width parameters ( NestYields )
We now allow to specify the NRERWidthsParameter that are used by nest to do all the calculations on recombination fluctuation etc, to go from pure yield to actual number of quanta produced. These parameters can be tuned to match our data. Leave dict empty to use defaults from NEST.
New option: return directly yields model output
We add an option to produce simulations turning off all statistical fluctuations and recombination fluctuation.
Handy to study response decoupling from other effects. Use with the option
return_yields_only
.New option: Beta yields threshold
Introduce an energy threshold for using the BetaYield ( like only >10keV ) - it's difficult to make a function that goes well also there, and nest gives good behavior at very low energies, setting manually the yields model to 0 at 0.3keV for LY and 0.01keV for CY. This is necessary for us to avoid having yield model going to infinity at these very low values. Use with the option
beta_yields_threshold
, default to 10keV.New option: Fix electric field for yields ( BetaYields )
We can now decide to fix the value of electric field that is used to calculate the yields, separately for gammas and betas, using
fix_beta_yield_field
andfix_gamma_yield_field
.Improvement: Lineage clustering logic
We now add the case of a gamma-phot interaction happening inside a photo-absorption (also gamma-phot) cluster. This additional interactions could be due to x rays emitted.
Bug fix: Lineage cluster classify
It does not matter what the creaproc was, if it is gamma-phot, give gamma yields.
Bug fix: no electrons if create_S2 is False
In the implementation of BetaYields, the treatment of create_S2 was missing. Now we correctly set to zero the production of electrons in the plugin if the create_S2 of the interaction is set to False.
Removal: recombination fluctuation model
The current implementation does not support anymore an empirical data motivated model for recombination fluctuation, as this is now done with all the necessary details by the nest getQuanta function.