-
Notifications
You must be signed in to change notification settings - Fork 21
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
Introduction of SegMIP BDT + other improvement on ECAL veto #1383
Conversation
We expect the tests to fail, both logs and histogram comparisons. |
this should be not related |
Nvm, the issue is in the prev line
Let me fix this typo |
eb8577a
to
16cd33d
Compare
16cd33d
to
11816e6
Compare
10f2a81
to
6bd297b
Compare
Lots of diffs in the validation, many actually come from a simple change that now the electron beam energy in the ecal veto is configurable, now the plots are correct (meaning consistent) for the signal. For the ecal pn, we need to change the config to be at 8 GeV. I also think we should run tracking in the validation configs. I can do that in another PR. I propose to do that next and have new golds / subversion change after that so then we can launch a BDT skim. |
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.
As usual, I mainly have some C++ technical comments. I think this implementation looks good and resolves two ( 2️⃣ ❗ ) issues so I'm happy its going to be merged.
I will continue to grumble about #1306 but leave that for future work.
a83aec8
to
8d1fcfc
Compare
I am updating ldmx-sw, here are the details.
What are the issues that this addresses?
Resolves #1287
Resolves #1305
We have shown the results of the software changes already at a SWAN meeting. The latest BDT that's now trained on the full E14 trigger skimmed data is included now too. This is to be shown at a SWAN meeting.
Future improvements in another PR will include factorizing the MIP tracking out of this class.
Check List