Should trigger decision names end in "Decision" rather than "Line"?

FYI @rmatev @sstahl @mvesteri

During my development of the HltEfficiencyChecker tool (see https://gitlab.cern.ch/lhcb/mooreanalysis/merge_requests/1 if interested) I had to plumb TupleToolTrigger onto the end of Moore. Currently, the dec reports have strings that end in Line. For instance you'd have a branch in a tuple called {particle}_Hlt1TwoTrackMVALine.

In run 2 this strings had the suffix Decision, and indeed TupleToolTrigger expects to see strings that end in Decision e.g. Hlt1TwoTrackMVADecision (see https://gitlab.cern.ch/lhcb/Analysis/blob/master/Phys/DecayTreeTupleTrigger/src/TupleToolTriggerBase.cpp#L145).

The question is this: should I change TupleToolTrigger to look in the dec reports for Lines, or should we keep the things same the same as run 2 and change the dec report writing to write out Decisions?

Related to Analysis!576 (merged)

Thinking back to when I first saw a tuple of LHCb data, I lean towards thinking Decision is more self-explanatory than Line.

I agree let's keep Decision, makes things easier.

I don't like Decision! It's not the line name. Then one has to remember to append Decision to the line name in order to retrieve the decision (e.g. when using the TISTOS tools). I can't count the number of times I've forgotten to do that.

(I expect we'll keep Decision, but I reserve the right to complain about it )

A brief comment on the user-side. Presumably it is easy for the tuple tool to fix the list of lines specified by the user so it doesn't matter whether they write Decision or not.

That's an option, but the problem of forgetting to append Decision is there whenever one interacts with the reports. One could modify the DecReports container to be agnostic instead, but… urgh.

@rmatev would you like to add anything to this discussion? Also @apearce is it a lot of effort to change things back to Decision? Is there a lot of places where this is hard coded/needs to be hard coded in? Does the fact that we use Line mean that you don't have to hard code in a lot of Decision strings everywhere? If so that might be a good reason to stick with Line.

tl;dr; it's subjective, so I think we should keep consistency with run 2.

The decision reports object contains decisions, so Hlt1XyzDecision makes sense (it's the decision of a line). Hlt1Xyz also makes sense, as "decision" can be implied. Hlt1XyzLine can also make sense but the Line suffix looks a bit redundant to me. In any case, in the tuple you want to have the some suffix like "decision" or "dec" because Hlt1Xyz[Line] tells nothing about what the column contains.

The changes won't be severe. I think all that's needed is ensuring Decision is appended to the line names during the Dec/SelReports creation, and then (ideally) testing the assumption of line names not ending in Decision and the assumption of report instances ending with it.

Ok, thanks folks. It seems we want to be consistent with run 2. So we have a choice to make w/ regards my HltEfficiencyChecker tool's MR/usage:

1. I merge in the changes to Analysis that ask TupleToolTrigger to look for strings in the decreports that end in Line rather than Decision, creating a follow-up issue to make all the necessary changes in Moore needed to switch back to Decision, which will include changing TupleToolTrigger back to looking for Decision. This will get my tool ready to use faster as it doesn't require any extra work, but it is a bit silly to change something just to change it back soon after.

2. Someone (me?) makes the changes needed to switch back to Decision, and I don't make the related changes in TupleToolTrigger. This makes more sense for changing TupleToolTrigger, but requires more effort and will take longer.

Personally I'd like to have a first iteration of HltEfficiencyChecker merged and therefore usable ASAP, but that is just me getting twitchy. If the tool can wait for however long it will take to change Line to Decision then perhaps we should do 2). But I'm not sure how long it will be before Allen want to start using the tool and we want to start making plots for the Allen vs Moore review?

Any thoughts @rmatev @apearce @sstahl @mvesteri?

I can take a stab at 2.

btw, I now wonder why we started suffixing line names with Line but I guess we can keep that for a bit

Thanks @rmatev!

@rjhunter I prepared a branch called rmatev-add-decision in LHCb, Phys and Moore. Can you check if that works for you? I'll open MRs soon

Thanks very much! I'm testing them now, I just had to add various recent changes to my master branches and the stack is taking some time to recompile. I will let you know how your changes work out.

Out of the box and changing "Line" back to "Decision" in TupleToolTriggerBase didn't work. I will have to investigate tomorrow.

@rmatev Actually my git got confused when I tried to put together your branch and my branch, and was quietly erasing your changes in your LHCb branch, which confused me for awhile. It was something uninteresting to do with merge vs. rebase. Once I got git to behave, it seems your changes do the trick. Thanks!

assigned to @rmatev

mentioned in commit LHCb@54272df7

mentioned in commit Phys@c20fb247

mentioned in commit 93415b72

mentioned in merge request Analysis!576 (merged)

mentioned in commit LHCb@288d397f

mentioned in merge request LHCb!2373 (merged)

mentioned in merge request Phys!662 (merged)

mentioned in merge request !389 (merged)

mentioned in commit 399ca58b

mentioned in issue #134 (closed)

mentioned in commit LHCb@fc4782ff

mentioned in commit Phys@8bb1348e

closed via merge request !389 (merged)

mentioned in commit 5d8ed411

mentioned in commit 913fae8d

mentioned in commit LHCb@f544d0f3

mentioned in commit LHCb@5ddd3432

mentioned in commit LHCb@66ea72cf

mentioned in commit LHCb@7cf3563d

mentioned in commit LHCb@e75a43a0