Page MenuHomeHEPForge

Add new MIPW example (requires a couple of new resonances in LauResonanceMaker)
ClosedPublic

Authored by mwhitehe on May 18 2021, 9:01 AM.

Diff Detail

Repository
rLAURA laura
Branch
mwhitehe_MIPW
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 194
Build 194: arc lint + arc unit

Event Timeline

  • Minor tidying of comments
  • Remove C style casts etc
mwhitehe changed the edit policy from "All Users" to "Laura (Project)".May 18 2021, 11:03 AM
mwhitehe added a project: Laura.
tlatham requested changes to this revision.May 18 2021, 3:25 PM

Many thanks for putting this together Mark, it's great to have an example that uses the MIPW.
I haven't yet tried running it but will try and do so later on, so these are just from a read through and all quite minor stuff.
(For one I've tried out the new "Suggest edit" feature, so hopefully it will be clear what I mean!)

examples/GenFitDpipi.cc
77

Probably better to either drop this line or put in some other hard-coded number that isn't zero, just so that things are reproducible.
In particular, if we want to use the examples as the basis for some integration tests then it is better to have them reproducible.
I notice that SimFitTask.cc has this same line, so perhaps you could modify that to match at the same time?
Eventually we probably want to allow the seed to be set by command-line option but that can be done as part of T122.

120–142

Seems unnecessary to create a vector and then fill a set from it, can just create the set and use it.

208–239

There seems to be some duplication here of certain things and I'm not sure if the right file names are being used in the right places.
Can you check against what is done in, e.g., GenFit3pi, and make sure everything matches?
I think I unified this across the various examples at some point, so they should all follow the same pattern.

This revision now requires changes to proceed.May 18 2021, 3:25 PM
  • Address comments from Tom

Thanks Mark, looks good, I'll merge it.

This revision is now accepted and ready to land.May 18 2021, 10:15 PM
tlatham changed the visibility from "All Users" to "Public (No Login Required)".May 18 2021, 10:26 PM