Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bound parameters visualization #230

Merged

Conversation

XiaocongAi
Copy link
Contributor

This PR includes a simple fix to the drawBoundParameters function.
Before this fix, the ellipse showing covariance of local position is displaced from the cone showing the covariance of momentum direction as below:
image
After the fix, they origin of the cone overlaps with the center of the ellipse as expected:
image

@acts-issue-bot acts-issue-bot bot added the Triage label Jun 2, 2020
@XiaocongAi XiaocongAi added Bug Something isn't working Component - Core Affects the Core module Impact - Minor Nuissance bug and/or affects only a single module labels Jun 2, 2020
@acts-issue-bot acts-issue-bot bot removed the Triage label Jun 2, 2020
@codecov
Copy link

codecov bot commented Jun 2, 2020

Codecov Report

Merging #230 into master will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #230      +/-   ##
==========================================
- Coverage   45.00%   44.99%   -0.01%     
==========================================
  Files         374      374              
  Lines       18697    18697              
  Branches     8841     8842       +1     
==========================================
- Hits         8414     8413       -1     
  Misses       4906     4906              
- Partials     5377     5378       +1     
Impacted Files Coverage Δ
...lude/Acts/Visualization/EventDataVisualization.hpp 42.46% <0.00%> (-1.37%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f289d34...cde7c6a. Read the comment docs.

Copy link
Contributor

@asalzburger asalzburger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be something with the lazy calculation of Eigen?

@asalzburger
Copy link
Contributor

Overwriting pathetic 0.01 % codcov patch complaint.

@asalzburger asalzburger merged commit 256ab0e into acts-project:master Jun 3, 2020
FabianKlimpel pushed a commit to FabianKlimpel/acts that referenced this pull request Jun 3, 2020
FabianKlimpel pushed a commit to FabianKlimpel/acts that referenced this pull request Jun 3, 2020
@paulgessinger paulgessinger added this to the v0.26.00 milestone Jun 11, 2020
paulgessinger pushed a commit to paulgessinger/acts that referenced this pull request Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Component - Core Affects the Core module Impact - Minor Nuissance bug and/or affects only a single module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants