Skip to content
Snippets Groups Projects

WIP: Track parameter dimensions

Closed WIP: Track parameter dimensions
TrackDimensions into master
6 unresolved threads
Closed Fabian Klimpel requested to merge TrackDimensions into master
6 unresolved threads

This MR increases the local track parameter dimensions to 6 and the global track parameter dimensions to 8. The additional parameter represents the timing. Additionally, this MR get rid of hard-coded dimension numbers and replaces them by a variable. Note: At the current state, this MR just shows all the parts of the code that need to be replaced/expanded in order to describe the timing. As soon as this is implemented, it will be re-included into the code and polished.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
Tests/Core/EventData/BoundParametersTests.cpp
83 83 // now create parameters on this surface
84 84 // l_x, l_y, phi, theta, q/p (1/p)
85 85 std::array<double, 5> pars_array = {{-0.1234, 9.8765, 0.45, 0.888, 0.001}};
86 TrackParametersBase::ParVector_t pars;
87 pars << pars_array[0], pars_array[1], pars_array[2], pars_array[3],
88 pars_array[4];
86 TrackParametersBase::ParVector_t pars
87 = TrackParametersBase::ParVector_t::Zero();
88 pars(0) = pars_array[0];
89 pars(1) = pars_array[1];
90 pars(2) = pars_array[2];
91 pars(3) = pars_array[3];
92 pars(4) = pars_array[4];
  • Paul Gessinger
    Paul Gessinger @pagessin started a thread on the diff
    • Tests/Core/CommonHelpers/include/Acts/Tests/CommonHelpers/FloatComparisons.hpp
    284 288 //
    285 289 auto orderOfMagnitude = std::sqrt(ref(row, row) * ref(col, col));
    286 290 if (std::abs(val(row, col) - ref(row, col)) >= tol * orderOfMagnitude) {
    291 if (val(row, col) == 0 && ref(row, col) == 0) {
    • Paul Gessinger
      Paul Gessinger @pagessin ·
      Owner

      Can you elaborate why this is a good idea? Maybe @hgraslan wants to comment.

    • Fabian Klimpel
      Fabian Klimpel @fklimpel ·
      Author Contributor

      Without this, you get an error in the case you compare 0 == 0

    • Hadrien Benjamin Grasland
      Hadrien Benjamin Grasland @hgraslan ·
      Developer

      Actually, this was by design.

      In the world of floating-point, comparing a result to a zero reference using a relative comparison method like this one is playing a very dangerous game, unless you are absolutely, positively sure that the result will always be exactly zero.

      If you want to allow it, a cleaner way is to replace the inequality above with a strict one.

    • Hadrien Benjamin Grasland
      Hadrien Benjamin Grasland @hgraslan ·
      Developer

      My general recommendation would be to use absolute comparisons if you have zeroes in your reference. But I can be convinced that covariance matrices are enough of a special case to warrant special treatment. After all, since these matrices mix unrelated physical quantities of wildly varying orders of magnitude, no single absolute threshold would be right for them, and a matrix of absolute tolerances would make tests unnecessarily verbose.

      Edited by Hadrien Benjamin Grasland
    • Fabian Klimpel
      Fabian Klimpel @fklimpel ·
      Author Contributor

      Since there is no calculation implemented for the new dimension yet, we have a column & row which is always 0. I will check, if this is still necessary as soon as there is some mathematics involved in the new dimension

    • Please register or sign in to reply
  • Paul Gessinger
    Paul Gessinger @pagessin started a thread on the diff
    • Tests/Core/EventData/ParameterSetTests.cpp
    509 loc0_loc1_phi_proj.block<3, 5>(0, 0) = loc0_loc1_phi_proj_def;
    510
    511 ActsMatrixD<4, 5> loc0_phi_theta_qop_proj_def;
    512 loc0_phi_theta_qop_proj_def << 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1, 0,
    513 0, 0, 0, 0, 1;
    514 ActsMatrixD<4, TrackParsDim> loc0_phi_theta_qop_proj
    515 = ActsMatrixD<4, TrackParsDim>::Zero();
    516 loc0_phi_theta_qop_proj.block<4, 5>(0, 0) = loc0_phi_theta_qop_proj_def;
    517
    518 ActsMatrixD<5, 5> loc0_loc1_phi_theta_qop_proj_def;
    519 loc0_loc1_phi_theta_qop_proj_def << 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1,
    520 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1;
    521 ActsMatrixD<5, TrackParsDim> loc0_loc1_phi_theta_qop_proj
    522 = ActsMatrixD<5, TrackParsDim>::Zero();
    523 loc0_loc1_phi_theta_qop_proj.block<5, 5>(0, 0)
    524 = loc0_loc1_phi_theta_qop_proj_def;
  • Paul Gessinger
    Paul Gessinger @pagessin started a thread on the diff
    • Tests/Core/Fitter/GainMatrixTests.cpp
    41 40 cylinder, 0, std::move(cov), -0.1, 0.45));
    42 41
    43 42 // Make dummy track parameter
    44 ActsSymMatrixD<Acts::NGlobalPars> covTrk;
    45 covTrk << 0.08, 0, 0, 0, 0, 0, 0.3, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1, 0,
    46 0, 0, 0, 0, 1;
    47 ActsVectorD<Acts::NGlobalPars> parValues;
    48 parValues << 0.3, 0.5, 0.5 * M_PI, 0.3 * M_PI, 0.01;
    43 ActsSymMatrixD<5> covTrk_def;
    44 covTrk_def << 0.08, 0, 0, 0, 0, 0, 0.3, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1,
    45 0, 0, 0, 0, 0, 1;
    46 TrackSymMatrix covTrk;
    47 covTrk.block<5, 5>(0, 0) = covTrk_def;
  • Paul Gessinger
    Paul Gessinger @pagessin started a thread on the diff
    • Tests/Core/CommonHelpers/include/Acts/Tests/CommonHelpers/FloatComparisons.hpp
    69 69 closeOrSmall(double reltol, double small)
    70 70 {
    71 71 return [=](double val, double ref) -> predicate_result {
    72 // Compare both directly
    73 if (val == ref) {
  • Hadrien Benjamin Grasland
    Hadrien Benjamin Grasland @hgraslan started a thread on the diff
    • Tests/Core/CommonHelpers/include/Acts/Tests/CommonHelpers/FloatComparisons.hpp
    69 69 closeOrSmall(double reltol, double small)
    70 70 {
    71 71 return [=](double val, double ref) -> predicate_result {
    72 // Compare both directly
    73 if (val == ref) {
    74 return true;
    75 }
    • Hadrien Benjamin Grasland
      Hadrien Benjamin Grasland @hgraslan ·
      Developer

      ...on the other hand, I would recommend against this change, as AFAIK any user of this relative comparison function can trivially be moved to either 1/set a smallness threshold or 2/use absolute comparisons instead.

    • Please register or sign in to reply
  • Fabian Klimpel added 457 commits

    added 457 commits

    Compare with previous version

    Toggle commit list
  • Fabian Klimpel closed

    closed

    • Please register or sign in to reply