improvement in charge calculation in trackseeding

[Jeet-bhai]

Closes: #1271
Closes: #1272

Briefly, what does this PR introduce?

It calculates the charge more correctly.

Before the change

events->Scan("CentralTrackSeedingResults.phi:CentralTrackSeedingResults.phi-atan2(MCParticles.momentum.y[0],MCParticles.momentum.x[0]):CentralTrackSeedingResults.charge","abs(CentralTrackSeedingResults.phi-atan2(MCParticles.momentum.y[0],MCParticles.momentum.x[0]))>0.5")
***********************************************************
*    Row   * Instance * CentralTr * CentralTr * CentralTr *
***********************************************************
*      457 *        0 * 3.1368908 * 6.2784049 *        -1 *
*      457 *        2 * 3.1409514 * 6.2824654 *        -1 *
*     1051 *        0 * 3.1248598 *  6.197864 *        -1 *
*     1541 *        2 * -2.652385 * -3.144119 *        -1 *
*     1593 *        0 * 2.1453850 * 3.1149456 *         1 *
*     1719 *        0 * 3.1414830 * 6.2790422 *        -1 *
*     1719 *        1 * 3.1376869 * 6.2752461 *        -1 *
*     1759 *        0 * 3.1079447 * 6.2340577 *        -1 *
*     1759 *        3 * 3.1407167 * 6.2668297 *        -1 *
*     2268 *        0 * -2.926911 * -3.144877 *        -1 *
*     2360 *        2 * -0.291468 * -3.144404 *        -1 *
*     2748 *        0 * 1.2643282 * 3.1381311 *        -1 *
*     3101 *        2 * 1.1373429 *  3.139108 *        -1 *
*     3460 *        4 * -0.901838 * -3.160678 *        -1 *
*     3464 *        0 * 3.1107549 * 6.2227986 *         1 *
*     3533 *        1 * 3.1413474 * 6.2821147 *        -1 *
*     3533 *        2 * 3.1400566 * 6.2808239 *        -1 *
*     3533 *        3 * 3.1406669 * 6.2814343 *        -1 *
*     3602 *        0 * 2.7184147 * 3.1398609 *         1 *
*     4080 *        2 * 0.1672719 * 1.5374061 *        -1 *
*     5168 *        2 * 1.1847635 * 3.1383791 *        -1 *
*     6023 *        2 * 0.1832947 * 3.1397418 *         1 *
*     6634 *        2 * 0.1372623 * 3.1390230 *        -1 *
*     6654 *        1 * 3.1401174 * 6.2715614 *        -1 *
*     6979 *        2 * 1.2488478 * 3.1390423 *        -1 *
Type <CR> to continue or q to quit ==> 
*     7302 *        2 * 0.0834536 * 3.1398086 *         1 *
*     7641 *        2 * 1.4545679 * 3.1399058 *         1 *
*     7896 *        0 * 3.1225175 * 6.2049581 *         1 *
*     8508 *        2 * -2.191989 * -3.143604 *         1 *
*     9033 *        2 * 1.2532205 * 3.1398001 *         1 *
*     9094 *        4 * -3.139707 * -6.280495 *        -1 *
*     9779 *        0 * 1.0651279 * 3.1089109 *        -1 *
*     9786 *        2 * -1.552465 * -3.143571 *        -1 *
***********************************************************
==> 33 selected entries

after the change:

events->Scan("CentralTrackSeedingResults.phi:CentralTrackSeedingResults.phi-atan2(MCParticles.momentum.y[0],MCParticles.momentum.x[0]):CentralTrackSeedingResults.charge","abs(CentralTrackSeedingResults.phi-atan2(MCParticles.momentum.y[0],MCParticles.momentum.x[0]))>0.5")
***********************************************************
*    Row   * Instance * CentralTr * CentralTr * CentralTr *
***********************************************************
*      457 *        0 * 3.1368908 * 6.2784049 *        -1 *
*      457 *        2 * 3.1409514 * 6.2824654 *        -1 *
*     1051 *        0 * 3.1248598 *  6.197864 *        -1 *
*     1719 *        0 * 3.1414830 * 6.2790422 *        -1 *
*     1719 *        1 * 3.1376869 * 6.2752461 *        -1 *
*     1759 *        0 * 3.1079447 * 6.2340577 *        -1 *
*     1759 *        3 * 3.1407167 * 6.2668297 *        -1 *
*     3464 *        0 * 3.1107549 * 6.2227986 *         1 *
*     3533 *        1 * 3.1413474 * 6.2821147 *        -1 *
*     3533 *        2 * 3.1400566 * 6.2808239 *        -1 *
*     3533 *        3 * 3.1406669 * 6.2814343 *        -1 *
*     4080 *        2 * 0.1672719 * 1.5374061 *        -1 *
*     6654 *        1 * 3.1401174 * 6.2715614 *        -1 *
*     7896 *        0 * 3.1225175 * 6.2049581 *         1 *
*     9094 *        4 * -3.139707 * -6.280495 *        -1 *
***********************************************************
==> 15 selected entries

What kind of change does this PR introduce?

• [x ] Bug fix (issue #__)
• New feature (issue #__)
• [x ] Documentation update
• Other: __

Please check if this PR fulfills the following:

• [ x] Tests for the changes have been added
• [x ] Documentation has been added / updated
• [x ] Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

no

Does this PR change default behavior?

no

[github-actions]

Capybara summary for PR 1281

• rec_dis_10x100_minQ2=1000_craterlake
• rec_dis_10x100_minQ2=1_craterlake
• rec_dis_18x275_minQ2=1000_craterlake
• rec_dis_18x275_minQ2=1_craterlake
• rec_dis_5x41_minQ2=1_craterlake
• rec_e_1GeV_20GeV_craterlake
• rec_pi_1GeV_20GeV_craterlake

[wdconinc]

With all respect to the work into this in a short time, I don't think this is ready for the February release (as was discussed and decided at the production meeting yesterday). Pushing for merging and including in the release (outside of the meetings we have for this) is not helpful and in my opinion a bit premature since there are outstanding change requests. The impact is 15 events / 10k, which from what I understand only solves half of the problem of incorrect seed, so I don't think this qualifies as a hot fix either.

[veprbl]

According to capybara, this seemingly doesn't translate to a significant improvement in the actual reconstructed tracks. The idea behind PR is solid, we should address the comments and merge this at some point.

[bschmookler]

I agree with Wouter that this doesn't need to be merged today. The problem is in about 15-20 seeds out of 20k seeds. So Jeet and I can work on this more next week.

Note that the seeds that show a reconstruction minus true phi close to 2*Pi is not an issue. That is just due to the discontinuity in phi definition over the negative x axis.

[wdconinc]

The idea behind PR is solid, we should address the comments and merge this at some point.

Agreed. Maybe by then the other half of the mismatched phi directions will be resolved too (if they are not just expected randoms).

[Jeet-bhai]

How about this?

int eicrecon::TrackSeeding::determineCharge(std::vector<std::pair<float,float>>& positions, const std::pair<float,float>& PCA, std::tuple<float,float,float>& RX0Y0) const
{

int charge = 1;

const auto& firstpos = positions.at(0);
auto hit_x = firstpos.first;
auto hit_y = firstpos.second;

auto xpos = PCA.first;
auto ypos = PCA.second;

float X0 = std::get<1>(RX0Y0);
float Y0 = std::get<2>(RX0Y0);

Acts::Vector2 radial(X0-xpos, Y0-ypos);
Acts::Vector2 hit(hit_x-xpos, hit_y-ypos);

Eigen::Matrix2d matrix;
matrix << radial(0), radial(1), hit(0), hit(1);
auto det = matrix.determinant();

if (det > 0) charge = -1;

return charge;
}

[veprbl]

Diff LGTM, capybara doesn't show statistically significant changes.