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 boundaries navigation #2534

Merged
merged 5 commits into from
Oct 25, 2023

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Oct 11, 2023

I encountered looping navigation between boundary surfaces as we do the lookup with overstep limit.

I believe we do not want to look for any boundaries that are behind us? But I might overlook something

Pulled this out of #2518

blocked by

@andiwand andiwand added this to the next milestone Oct 11, 2023
@paulgessinger
Copy link
Member

What if we slightly overstep onto a boundary surface? Then we want to go backwards to get to it, don't we?

@andiwand
Copy link
Contributor Author

What if we slightly overstep onto a boundary surface? Then we want to go backwards to get to it, don't we?

That should still be the case. When we target a boundary the navigator will try to reach it and handle overstepping. But when we look up boundaries after we changed volumes we do not want to look back I think

@github-actions github-actions bot added the Component - Core Affects the Core module label Oct 11, 2023
paulgessinger
paulgessinger previously approved these changes Oct 13, 2023
@paulgessinger
Copy link
Member

This fails the global $\chi^2$ fitter test. Can you have a look @andiwand?

@andiwand andiwand added the 🛑 blocked This item is blocked by another item label Oct 16, 2023
@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

Merging #2534 (2849632) into main (630fc20) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2534   +/-   ##
=======================================
  Coverage   49.64%   49.64%           
=======================================
  Files         471      471           
  Lines       26632    26632           
  Branches    12237    12237           
=======================================
  Hits        13222    13222           
  Misses       4742     4742           
  Partials     8668     8668           
Files Coverage Δ
Core/src/Geometry/TrackingVolume.cpp 47.64% <100.00%> (ø)

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

kodiakhq bot pushed a commit that referenced this pull request Oct 16, 2023
While debugging #2534 I encountered a few problems with the GX2F test which I try to fix here.
- make volume big enough to fit all the surfaces
- introduce envelope
- remove volume material
- lower case variable name
@andiwand andiwand removed the 🛑 blocked This item is blocked by another item label Oct 25, 2023
@kodiakhq kodiakhq bot merged commit f84610d into acts-project:main Oct 25, 2023
@andiwand andiwand deleted the fix-boundaries-navigation branch October 25, 2023 08:58
@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
Component - Core Affects the Core module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants