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: Fix vertexing for physmon ckf tracking #2547

Merged
merged 3 commits into from
Oct 17, 2023

Conversation

andiwand
Copy link
Contributor

associatedParticles should not be used after CKF as we cannot guarantee that they will line up with tracks coming from the CKF. this way the writer will try to associate the particles via tracks itself.

@andiwand andiwand added this to the next milestone Oct 16, 2023
@github-actions github-actions bot added the Infrastructure Changes to build tools, continous integration, ... label Oct 16, 2023
@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

Merging #2547 (3f4f80a) into main (ecc68bc) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

❗ Current head 3f4f80a differs from pull request most recent head 43781b4. Consider uploading reports for the commit 43781b4 to get more accurate results

@@           Coverage Diff           @@
##             main    #2547   +/-   ##
=======================================
  Coverage   49.76%   49.76%           
=======================================
  Files         466      466           
  Lines       26326    26326           
  Branches    12097    12097           
=======================================
  Hits        13101    13101           
  Misses       4624     4624           
  Partials     8601     8601           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@felix-russo felix-russo left a comment

Choose a reason for hiding this comment

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

Isn't this conflicting with the name of the physmon procedure truth_estimated/truth_smeared? Or am I missing something?

@andiwand
Copy link
Contributor Author

Isn't this conflicting with the name of the physmon procedure truth_estimated/truth_smeared? Or am I missing something?

that refers to the seeder not the track finding

felix-russo
felix-russo previously approved these changes Oct 17, 2023
@kodiakhq kodiakhq bot merged commit 69e2190 into acts-project:main Oct 17, 2023
53 of 54 checks passed
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Oct 17, 2023
@andiwand andiwand deleted the fix-physmon-ckf-vertexing branch October 17, 2023 11:04
kodiakhq bot pushed a commit that referenced this pull request Oct 25, 2023
Currently our Fatras simulation does not remember the surface particles were created on. This is a problem for the navigation as we cannot know if a close by surface was already encountered or not.

I propose to add a reference surface to the Fatras `Particle` which can be carried through in the propagation and then be used as a starting point for a new propagation.

We will also have to decide if a photon conversion on a sensitive surface should result in a hit or not. But this should be done in a separate PR if possible.

blocked by
- #2547
- #2549
@paulgessinger paulgessinger modified the milestones: next, v30.3.0 Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes Performance Fails Athena tests This PR causes a failure in the Athena tests Infrastructure Changes to build tools, continous integration, ...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants