refactor!: Rewrite navigation #2625
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andiwand
commented
Nov 3, 2023
This was referenced Nov 3, 2023
github-actions
bot
added
the
Infrastructure
📊: Physics performance monitoring for 34a290ephysmon summary
|
acts-project-service
added
the
Breaks Athena build
andiwand
added a commit
to andiwand/acts
that referenced
this pull request
Dec 5, 2023
paulgessinger
pushed a commit
that referenced
this pull request
Dec 5, 2023
As the templating of the representing object was rather a burden in #2625 I propose to remove it all together and compose the intersection if necessary. Here I used a `std::pair` in the `Navigator` for composition for now which may be not optimal but it works as it should for now
LaraCalic
added a commit
to LaraCalic/acts
that referenced
this pull request
Dec 11, 2023
apparently otherwise the random engine can be different on different platforms which I observed in acts-project#2625 with macos at the same time I hardcoded the type for the `std::uniform_int_distribution`. apparently this can also make a difference for the random value
LaraCalic
added a commit
to LaraCalic/acts
that referenced
this pull request
Dec 18, 2023
apparently otherwise the random engine can be different on different platforms which I observed in acts-project#2625 with macos at the same time I hardcoded the type for the `std::uniform_int_distribution`. apparently this can also make a difference for the random value
kodiakhq bot
pushed a commit
that referenced
this pull request
Dec 19, 2023
After #2625 turned out to be harder than initially thought I tried to factor out some changes to get them into main already. Summary of the changes I made - `pLimit` = path limit -> far limit = `farLimit` - `oLimit` = overstep limit -> near limit = `nearLimit` - replaced `DetectorNavigator` iterators with indices
|
I broke this one up into a few follow up PRs
I fear I cannot make this fast enough to compete with the existing navigation. I will rather shift my focus on the new geometry and its navigation next year closing |
LaraCalic
pushed a commit
to LaraCalic/acts
that referenced
this pull request
Feb 10, 2024
After acts-project#2625 turned out to be harder than initially thought I tried to factor out some changes to get them into main already. Summary of the changes I made - `pLimit` = path limit -> far limit = `farLimit` - `oLimit` = overstep limit -> near limit = `nearLimit` - replaced `DetectorNavigator` iterators with indices
EleniXoch
pushed a commit
to EleniXoch/acts
that referenced
this pull request
May 31, 2024
Backporting more changes from acts-project#2625 to main. This is a followup to acts-project#2768 Summary - remove overstepping from steppers - use surface tolerance for geometry lookups - add self consistency navigation test
Matthewharri
pushed a commit
to Matthewharri/acts
that referenced
this pull request
Jun 18, 2024
Backporting more changes from acts-project#2625 to main. This is a followup to acts-project#2768 Summary - remove overstepping from steppers - use surface tolerance for geometry lookups - add self consistency navigation test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
this is experimental
I discovered too many navigation issues lately and decided to rewrite it in a way that is (hopefully) more robust and which fails fast. In principle I just switched the existing navigation to a single intersection stream rather than having the boundary, layer, surface hierarchy.
incorporates:
blocked by
SurfaceReachedaborter #2603CuboidVolumeBuilder#2681MaterialInteractor#2691