Page MenuHomeHEPForge

Add EvtEtaLLPiPi model.
ClosedPublic

Authored by jback on Aug 21 2020, 7:48 PM.

Details

Summary

Add EvtEtaLLPiPi model for eta' -> l l pi pi decays (l = e or mu), for https://phab.hepforge.org/T109.

Test Plan

Model has been tested using testDecayModel (in the jsonTest branch).

Diff Detail

Repository
rEVTGEN evtgen
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 81
Build 81: arc lint + arc unit

Event Timeline

jback changed the edit policy from "All Users" to "Restricted Project (Project)".Aug 21 2020, 7:49 PM
jback added a project: Restricted Project.
jback requested review of this revision.Aug 21 2020, 7:51 PM
jback changed the visibility from "All Users" to "Public (No Login Required)".Sep 1 2020, 3:17 PM
tlatham requested changes to this revision.Oct 2 2020, 10:22 PM

Hi @jback, sorry it's taken me so long to review this.
You can find here some technical points.
I'll try to compare the physics against the referenced paper next week, although I guess this was checked during the LHCb review?
You mention also testing it using testDecayModel but I can't find the json file(s) for this model. Could you include them in the next revision?

EvtGenModels/EvtEtaLLPiPi.hh
30

Can you please run clang-format (preferably version 10.0.0) on these few files when submitting the next version?
Will add mention of this in the CONTRIBUTING docs.

History.txt
15

Thanks for remembering to update this file! I shall also mention this in the CONTRIBUTING docs.
Would be good to reference here the Maniphest tasks and/or the Differential revisions associated with each change.
I think there's also been one other bug fix (T103) that we should probably include here.

src/EvtGenModels/EvtEtaLLPiPi.cpp
55

Would be better to give these values in the header.
Then this constructor can be removed from here and just be '=default' in the header.

This revision now requires changes to proceed.Oct 2 2020, 10:22 PM
  • Update EvtEtaLLPiPi model according to D34 suggestions.
jback marked 3 inline comments as done.Oct 3 2020, 3:08 PM

I'll try to compare the physics against the referenced paper next week, although I guess this was checked during the LHCb review?

Yes, Aleksei checked that the code changes produced distributions that matched the plots shown at the Gauss simulation meeting:

https://indico.cern.ch/event/876000

Relevant comment in code review:

https://gitlab.cern.ch/lhcb/Gauss/-/merge_requests/626#note_3723807

The plots looked OK for my tests as well.

You mention also testing it using testDecayModel but I can't find the json file(s) for this model. Could you include them in the next revision?

I've added the json files to the jsonTest branch: test/jsonFiles/EtaEEPiPi.json and EtaMuMuPiPi.json. This revision does not include the json testing code.

History.txt
15

Have added maniphest and differential links for code review in the history file, since version 2.0.0.

This comment was removed by jback.

Hi @jback, many thanks for the changes and apologies for the delay in responding to them.
I think this looks good to go now.
Unless @kreps has any comments I think you can go ahead and "land" it onto master.

This revision is now accepted and ready to land.Oct 30 2020, 11:16 AM
This revision was automatically updated to reflect the committed changes.