Skip to content
Commit 4315f7c1 authored by Sebastien Ponce's avatar Sebastien Ponce
Browse files

Converted Links input and output to functional in PrTrackAssociator

PrTrackAssociator is now believed to be fully functional.
parent 054ebaa3
Pipeline #705283 passed with stage
in 1 minute and 12 seconds
  • Maintainer

    @sponce would you expect this commit to have an effect on the association? I am observing differences in the Brunel logs between a reference file update done yesterday against https://lhcb-nightlies.cern.ch/logs/checkout/nightly/lhcb-lcg-dev4/809/Rec/ which included Rec!!406 (merged) but before this commit. See today's diffs at https://lhcb-nightlies.cern.ch/logs/tests/nightly/lhcb-head/2147/x86_64-centos7-gcc8-opt/Brunel/

  • Author Owner

    It could be that I fixed a small bug on the way. I thought it would not be visible, but this line of TrackAssociator :

    bool ttOK = ((*it).nTT + 2 > total.nTT);

    was executed with nTT being a float while it's now an int. The floats were initialized with 0 and always only added 1.0, so I would have said the comparison with floats would work, but now that you mention the changes and that they are only improving the numbers, I wonder if it can be that for equal values, we got different last bit

    Is it worth checking ?

  • Maintainer

    How hard is it to check? How is nTT used for Upgrade (for UT???)?

    Or to put it another way, how confident are you that, apart from this, behaviour is unchanged?

  • Author Owner

    Not so hard to check, so I suppose I'll do it as I have no clue what the answer is to the usage question.

    About confidence : in principle my changes should have been purely technical and not changing a bit of the output. Except for this float to int change added afterward. This is why I'm suspecting it.

  • Author Owner

    I can confirm that reverting commit 054ebaa3 "Used integers to count tracks in PrTrackAssociator" removes all discrepancies in the Checkers. Do you think I should put it in a separate merge request then ?

  • Maintainer

    Yes, it would be better to keep the MRs separate.

  • Sebastien Ponce @sponce

    mentioned in merge request !1411 (merged)

    ·

    mentioned in merge request !1411 (merged)

    Toggle commit list
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment