Skip to content
This repository has been archived by the owner on Jun 5, 2024. It is now read-only.

S1 scintillation timing from NEST #173

Merged
merged 26 commits into from
Jun 28, 2021
Merged

S1 scintillation timing from NEST #173

merged 26 commits into from
Jun 28, 2021

Conversation

terliuk
Copy link
Contributor

@terliuk terliuk commented Jun 17, 2021

What is the problem / what does the code in this PR do
Introduces new mode for S1 shape where timing is done from timing spline, and scintillation time is received directly from nestpy which will address issue #165

@terliuk
Copy link
Contributor Author

terliuk commented Jun 18, 2021

here is the example of running it : https://github.com/XENONnT/analysiscode/blob/master/s1_pulse_shape/S1_WFsim_pulse_shape_NEST_example.ipynb
One more comment - we might need to re-adjust epix instruction handling, as we need excitons, number of emitted photons and local field at the point of the energy deposit. @ramirezdiego has some ideas there, so likely we will make a "tandem" PR for both epix and wfsim.

@terliuk terliuk marked this pull request as ready for review June 18, 2021 12:44
@skazama
Copy link

skazama commented Jun 21, 2021

Hi @terliuk, I cannot comment about epix, but I had a look at your code and it looks fine. Also I managed to run the notebook with your branch and could generate Kr83m S1 signals without errors, so maybe we can merge this? (or do we have to wait for @ramirezdiego's ideas?)

@ramirezdiego
Copy link
Collaborator

ramirezdiego commented Jun 21, 2021

Hi @skazama. The code works fine indeed (thanks for jumping in!), but in a recent chat with @terliuk I suggested that it would be better to push the relevant changes to epix before merging this PR; i.e., to output the required information in the instructions (efield, number of excitons...).

This requires to change the instructions format also in WFSim: either by adding another one, with these extra columns, or adding the columns to the default one, but make epix always print out some dummy number for them. In case this wasn't clear: epix reads the format directly from WFSim: https://github.com/XENONnT/epix/blob/43e7e31a05d0ff52863c2017f367bff7eb859312/epix/io.py#L202 This would be the missing part of this PR.

Edit: I had missed the last comment by @terliuk, sorry. But okay, we seem to agree on pushing the epix fix at the same time/right before.

@terliuk
Copy link
Contributor Author

terliuk commented Jun 21, 2021

after recent changes, i need to do extensive testing again, confirming that it works as intended

@zhut19 zhut19 merged commit b204045 into master Jun 28, 2021
@zhut19 zhut19 deleted the s1_nest_model branch June 28, 2021 15:53
@zhut19 zhut19 mentioned this pull request Aug 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants