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

Update EcalVeto BDT for 8 GeV + change hardcoded energy #1287

Closed
tvami opened this issue Apr 20, 2024 · 6 comments · Fixed by #1383
Closed

Update EcalVeto BDT for 8 GeV + change hardcoded energy #1287

tvami opened this issue Apr 20, 2024 · 6 comments · Fixed by #1383
Assignees

Comments

@tvami
Copy link
Member

tvami commented Apr 20, 2024

Is your feature request related to a problem? Please describe.

Currently the ECal Veto BDT is optimized for 4 GeV.
Also just a note that the energy is hardcoded in the code here:
https://github.com/LDMX-Software/Ecal/blob/trunk/src/Ecal/EcalVetoProcessor.cxx#L276

Describe the solution you'd like

After the presentation on Monday we plan to update the ECal Veto BDT to the 8 GeV version.
Also make the energy configurable

Additional context

See the talk at the SWAN meeting
https://indico.fnal.gov/event/64332/#18-8-gev-bdt-studies-with-the

@tvami tvami self-assigned this Apr 20, 2024
@tomeichlersmith
Copy link
Member

I'm guessing this is connected to another issue I transferred over from the Ecal repo: #1305

If not, we can leave that one alone, but if it is then we can close two issues :)

@tomeichlersmith
Copy link
Member

I also want to plug #1306 if you're going to already be touching things.

Might be helpful for leaving the old-BDT implementation (with fewer inputs) in a separate processor (maybe named "GabrielleBDT" instead of SingleElectronTargetBDT as mentioned in the linked issue). This could also resolve @bryngemark 's processing time concerns since we could calculate the shower features in large batch and only do tracking+BDT after-the-fact on trigger skimmed samples or something.

@danyi211
Copy link
Contributor

danyi211 commented May 6, 2024

I'm guessing this is connected to another issue I transferred over from the Ecal repo: #1305

If not, we can leave that one alone, but if it is then we can close two issues :)

Yes, this is the new seg-MIP BDT we plan to implement :)

@tvami
Copy link
Member Author

tvami commented May 6, 2024

OK so today's SWAN meeting I think the agreement was that we'll do 2 PRs:

  • first just update so the physics is good with the new BDT
  • then make all the changes so that the computational aspect is good (i.e. separate MIP tracking)

Not sure about what to do with Gabrielle BDT, I was thinking of just overwriting the current one, but @tomeichlersmith do you see any reason to keep the old one as a separate producer?

@tomeichlersmith
Copy link
Member

Just for comparison, but it seems like (especially after this SWAN meeting) that everyone will step forward to the new one in which case I'm happy to just overwrite the current one. Folks can run with an previous version of ldmx-sw if they want to get the results of gabrielle.

@vdutta
Copy link

vdutta commented May 6, 2024

Btw, according to this list the new BDT should be called Humberto (!) but maybe we should update to the 2024 set of hurricane names and call it Helene instead :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants