Implementation of the Tprev/Tnext task.

[leandrofloyd]

Task to calculate and store the values of time difference between the Master Start hit and the previous/next hit.

---

Checklist:

• [x] Rebased against dev branch
• [x] My name is in the resp. CONTRIBUTORS/AUTHORS file
• [x] Followed the pull request guidlines and the Git workflow
• [x] Followed the seven rules of great commit messages

[leandrofloyd]

Thanks for your PR.

Besides some minor issues I mention below, from my understanding by reading the code, some values of tprev and tnext are reserved for special meanings. So if the value is equal to 5e8, it means something. If the value is equal to 5e7, it means other things. I'm not saying this is the wrong way but clearly there is a better way to do this. For example, you could use an enumerator to indicate the validity of the value:

enum class TprevTnextStatus
{
    valid,
    ms_hit,
    one_hit,
    bad_record
};

and then store this enumerator with your tprev or tnext values.

I will try to do that, good idea, thanks.

[leandrofloyd]

The question which values we should use to indicate invalid time values deserves some consideration.

Fully agree, this is one of the things I really want to be discussed here before the code is accepted.

I think people will mostly use Tprev and Tnext in cuts where they select events which are isolated: if (Tprev>1000 && Tnext > 1000) { // Fill your histograms or whatever } The default values you use for ((sampHits < 1) || (sampmsHits != 1)) are nice invalid values which will clearly indicate an unsafe event.

However, all of your other failure values are big positive numbers, claiming that there is no event for at least half a second around the one in question. I think this is overly optimistic.

You are right here, I just used the numbers I used based on the test cases I used, but we can discuss what is better for general purposes.

For an initial value of fTPrev, you could remember the value fLastWrts=GetWRTS()-10*uint64_t(tpn) (with your sign convention) of the last Exec call in a member variable. Then your fTPrev simply is GetWrts()-fLastWrts(). (If fLastWrts has never been set, just invalidate fTPrev by setting it negative.)

I really like your idea, but I will need more time to fully understand, maybe we can discuss that at GSI, the problem is, I am out on vacation for 3 weeks starting tomorrow (I was not expecting to have 5, 6 weeks of wait for the "Offset part" of the code).

Your fTnext is more troublesome, because with a single pass over the tree you can't know what comes next. A defensible value would be (offset-1)*10.0. After all, it took at least that long between the trigger which caused the readout to hit the sampler and the sampler actually bank switching (or whatever samplers do) for the readout.

Need more time here again to process the information.

Are you aware that the sampler clock wraps around every 43 seconds, and are ok with your difference values being widely off whenever that happens?

Yes, initially I made some checks and corrections related to that, but, again, maybe this is a good thing to discuss with you in person, but, again (again), I am going on vacation tomorrow, so we can discuss that after I return.

Thank you very much for this discussion, this is the kind of thing I was expecting to discuss here.

[jose-luis-rs]

Hello @leandrofloyd Do you plan to do more modifications? Cheers

[leandrofloyd]

Hello @jose-luis-rs Yes, I will have some modifications yet. Cheers

[leandrofloyd]

@jose-luis-rs @klenze @YanzhaoW Please check this new code. There are some more things I want to discuss, like the values for the cases, the use of WR, etc... But please take a look at how the code looks now and we can start discussions.

[leandrofloyd]

This version was made after a meeting with @klenze where he explained some of his comments and presented me with some ideas. I tried to implement every suggestion (if the code change doesn't change completely the part of the suggestion, of course).

[leandrofloyd]

@klenze and @YanzhaoW I hope that this version fulfils everything that both of you have asked for. @klenze did I understand your comments correctly?

[klenze]

Great, thanks!

@jose-luis-rs: This is ready for merging as far as I am concerned.

[jose-luis-rs]

Thanks!