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: full chain vertexing #1299

Merged
merged 87 commits into from
Aug 9, 2022

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Jun 29, 2022

fix full chain odd vertexing added here #1286

some early discussion here https://mattermost.web.cern.ch/acts/pl/m6psyge3apgcucesrgojeaajny

  • output track parameters from TrackFindingAlgorithm
  • refactor error handling in *VertexFinderAlgorithm
  • rewire default whiteboard names in python examples
  • use IVF in full_chain_odd.py with 2 muons (which should give us an actual vertex)

@andiwand andiwand added the 🚧 WIP Work-in-progress label Jun 29, 2022
@codecov
Copy link

codecov bot commented Jun 29, 2022

Codecov Report

Merging #1299 (a684ad4) into main (0352563) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main    #1299   +/-   ##
=======================================
  Coverage   47.76%   47.76%           
=======================================
  Files         380      380           
  Lines       20164    20164           
  Branches     9387     9387           
=======================================
  Hits         9632     9632           
  Misses       4083     4083           
  Partials     6449     6449           
Impacted Files Coverage Δ
...clude/Acts/Vertexing/AdaptiveMultiVertexFitter.ipp 46.00% <0.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@andiwand
Copy link
Contributor Author

andiwand commented Jul 5, 2022

@andiwand andiwand marked this pull request as ready for review July 6, 2022 13:36
@andiwand andiwand removed the 🚧 WIP Work-in-progress label Jul 6, 2022
@andiwand andiwand added this to the next milestone Jul 6, 2022
Copy link
Contributor

@AJPfleger AJPfleger left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@AJPfleger AJPfleger added the Impact - Minor Nuissance bug and/or affects only a single module label Jul 7, 2022
@andiwand
Copy link
Contributor Author

andiwand commented Aug 3, 2022

sadly this still does not work. I tried to use more particles for each event. I suspected that missing seeds for one of the particles would put the vertex fit in a bad position. I think this is still true but since there are so many duplicates for each track the order in which they go into the vertexing could still result into the same problem

the good news is that using #1363 seems to eliminate all the problems

@andiwand
Copy link
Contributor Author

andiwand commented Aug 4, 2022

after merging #1363 the full chain vertexing works now

@andiwand
Copy link
Contributor Author

andiwand commented Aug 5, 2022

ready @paulgessinger ?

@paulgessinger
Copy link
Member

Looks ok, let's go with IVF first and look into AMVF. Do you know if AMVF runs into errors in the current status?

@andiwand
Copy link
Contributor Author

andiwand commented Aug 5, 2022

Looks ok, let's go with IVF first and look into AMVF. Do you know if AMVF runs into errors in the current status?

AMVF will produce an error in the current scenario. I guess it is because we loose one of the muons and then fail to fit the vertex. with 4 muons AMVF works but IVF even works with 2 but I am not sure why

@andiwand andiwand removed the 🚧 WIP Work-in-progress label Aug 9, 2022
@kodiakhq kodiakhq bot merged commit 884d342 into acts-project:main Aug 9, 2022
@andiwand
Copy link
Contributor Author

andiwand commented Aug 9, 2022

@github-actions github-actions bot removed the automerge label Aug 9, 2022
@andiwand andiwand deleted the fix-full-chain-vertexing branch August 9, 2022 12:37
@paulgessinger paulgessinger modified the milestones: next, v20.0.0 Aug 16, 2022
@paulgessinger paulgessinger added the backport develop/v19.x Backport this PR to the v19.x series label Aug 16, 2022
acts-project-service pushed a commit that referenced this pull request Aug 16, 2022
fix full chain odd vertexing added here #1286

some early discussion here https://mattermost.web.cern.ch/acts/pl/m6psyge3apgcucesrgojeaajny

- output track parameters from `TrackFindingAlgorithm`
- refactor error handling in `*VertexFinderAlgorithm`
- rewire default whiteboard names in python examples
- use IVF in `full_chain_odd.py` with 2 muons (which should give us an actual vertex)

(cherry picked from commit 884d342)
kodiakhq bot pushed a commit that referenced this pull request Aug 16, 2022
Backport 884d342 from #1299.
---
fix full chain odd vertexing added here #1286

some early discussion here https://mattermost.web.cern.ch/acts/pl/m6psyge3apgcucesrgojeaajny

- output track parameters from `TrackFindingAlgorithm`
- refactor error handling in `*VertexFinderAlgorithm`
- rewire default whiteboard names in python examples
- use IVF in `full_chain_odd.py` with 2 muons (which should give us an actual vertex)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport develop/v19.x Backport this PR to the v19.x series 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