Skip to content
Snippets Groups Projects

Fix aliasing bug in newCovarianceMatrix method

All threads resolved!

The PatternTrackParameters class conains a method by the name of newCovarianceMatrix which updates the covariance matrix of the object it is called on according to the covariance matrix in a second set of parameters and a Jacobian matrix. The calculation performed is JCJ^T. In many cases where this method is called, the object on which it is called is the same as the object given as the right hand side of the operation. This means that the covariance matrices alias, leading to possibly incorrect results. It also invites behaviour where the calculations are done using invalid covariance matrices.

This commit fixes these issues by changing the signature of the method to accept a covariance matrix directly instead of a second set of track parameters. This makes it harder, albeit not impossible, to alias the covariance matrices as described earlier. It also solves the invalid covariance matrix problem described in !37151 (merged).

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
  • CI Result SUCCESS (hash 7a53e2b1)

    Athena AthSimulation AthGeneration AnalysisBase
    externals
    cmake
    make
    required tests
    optional tests

    Full details available on this CI monitor view
    Athena: number of compilation errors 0, warnings 0
    AthSimulation: number of compilation errors 0, warnings 0
    AthGeneration: number of compilation errors 0, warnings 0
    AnalysisBase: number of compilation errors 0, warnings 0
    📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 22135]

  • added 1 commit

    • b64418b1 - Fix aliasing bug in newCovarianceMatrix method

    Compare with previous version

  • This merge request affects 3 packages:

    • InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData
    • Tracking/TrkEvent/TrkPatternParameters
    • Tracking/TrkExtrapolation/TrkExRungeKuttaPropagator

    Adding @goetz ,@amorley ,@gavrilen ,@sroe ,@jsandesa as watchers

  • Christos Anastopoulos resolved all threads

    resolved all threads

  • CI Result SUCCESS (hash b64418b1)

    Athena AthSimulation AthGeneration AnalysisBase
    externals
    cmake
    make
    required tests
    optional tests

    Full details available on this CI monitor view
    Athena: number of compilation errors 0, warnings 0
    AthSimulation: number of compilation errors 0, warnings 0
    AthGeneration: number of compilation errors 0, warnings 0
    AnalysisBase: number of compilation errors 0, warnings 0
    📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 22151]

  • added 1 commit

    • c119a1a0 - Fix aliasing bug in newCovarianceMatrix method

    Compare with previous version

  • This merge request affects 3 packages:

    • InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData
    • Tracking/TrkEvent/TrkPatternParameters
    • Tracking/TrkExtrapolation/TrkExRungeKuttaPropagator

    Adding @goetz ,@amorley ,@gavrilen ,@sroe ,@jsandesa as watchers

  • CI Result SUCCESS (hash c119a1a0)

    Athena AthSimulation AthGeneration AnalysisBase
    externals
    cmake
    make
    required tests
    optional tests

    Full details available on this CI monitor view
    Athena: number of compilation errors 0, warnings 0
    AthSimulation: number of compilation errors 0, warnings 0
    AthGeneration: number of compilation errors 0, warnings 0
    AnalysisBase: number of compilation errors 0, warnings 0
    📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 22208]

  • Changes look ok to me, but asking L2 to take a look to double check the changes with the Jacobian calculations.

    Cheers Nathan (L1)

  • Conflicts, unfortunately.

    • Walter
  • added 397 commits

    Compare with previous version

  • This merge request affects 3 packages:

    • InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData
    • Tracking/TrkEvent/TrkPatternParameters
    • Tracking/TrkExtrapolation/TrkExRungeKuttaPropagator

    Adding @goetz ,@amorley ,@gavrilen ,@sroe ,@jsandesa as watchers

  • CI Result SUCCESS (hash 8919d760)

    Athena AthSimulation AthGeneration AnalysisBase
    externals
    cmake
    make
    required tests
    optional tests

    Full details available on this CI monitor view
    Athena: number of compilation errors 0, warnings 0
    AthSimulation: number of compilation errors 0, warnings 0
    AthGeneration: number of compilation errors 0, warnings 0
    AnalysisBase: number of compilation errors 0, warnings 0
    📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 22378]

  • All clean now, approving (L1).

  • added review-approved label and removed review-pending-level-1 label

  • mentioned in commit c2300f44

  • Please register or sign in to reply
    Loading