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

YZ 2D Simulation and Separate Electron Lifetime Simulation #611

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

rennney
Copy link
Contributor

@rennney rennney commented Jul 24, 2023

Add configurations to support new WireCell module to configure eLifetime per TPC

Currently split is configured based on anode dimensions. However, it can be possible to extend the functionality to add custom input / or add separate module depending on required input in the future.

This PR should be merge only with new version of WireCell

This PR suppose to break several upstream features, since it now outputs 8 simchannel versions (2 for each TPC). Numbering should be as following 0-1 : TPCEE , 2-3 : TPCEW , 4-5 : TPCWE , 6-7 : TPCWW. Contact me if this is not working correctly.
For this test I add @gputnam and @SFBayLaser as reviewers. [I will let you know as soon as new WireCell version is available]

Although I did several tests to make sure WC module and configurations are working correctly I suggest more validation to ensure eLifetime is correctly set for appropriate TPC.

Add configurations to support new WireCell module to comfigure eLifetime per TPC
@rennney rennney added breaking this change will break backward compatibility in some way new CI ref this change will change the output of integration tests labels Jul 24, 2023
@rennney rennney requested review from gputnam and SFBayLaser July 24, 2023 15:22
add file with graph defenition
Copy link
Contributor

@gputnam gputnam left a comment

Choose a reason for hiding this comment

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

Hi Sergey, thank you for this.

I'm going to request what I brought up in the meeting on Tuesday. If we merge this into our mainline detector simulation, it will break downstream code right now.

Either this needs to be put into a special configuration, or it needs to make a single SimChannel object, or we need to modify that downstream code.

I believe both CAFMaker and the Calibration NTuple-r would need modifications.

@rennney
Copy link
Contributor Author

rennney commented Aug 3, 2023

Hi, @gputnam as was decided during the meeting it is straightforward to sum all simchannels upstream at the moment . The summation from WC will come later. Since I am not an expert in the upstream code I hope you or @SFBayLaser could add required corrections to this PR or create an extra one.

@mmrosenberg mmrosenberg marked this pull request as draft September 9, 2023 21:15
@jzennamo jzennamo changed the title Add eLifetime split per TPC YZ 2D Simulation and Separate Electron Lifetime Simulation Jan 17, 2024
@jzennamo jzennamo self-assigned this Jan 17, 2024
@mmrosenberg mmrosenberg marked this pull request as ready for review January 17, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking this change will break backward compatibility in some way new CI ref this change will change the output of integration tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants