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.

Sign up for GitHub

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

Jump to bottom

refactor: Remove forceIntersectBoundaries from Navigator #3238

Closed
wants to merge 5 commits into from
Closed

refactor: Remove forceIntersectBoundaries from Navigator #3238

wants to merge 5 commits into from

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented May 30, 2024 •
edited
Loading

This is an attempt to drop a Navigator hotfix from the past assuming that it has been fixed by one of the Navigator refactors.

blocked by

@andiwand andiwand added this to the next milestone May 30, 2024
@github-actions github-actions bot added the Component - Core Affects the Core module label May 30, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented May 30, 2024 •
edited
Loading

📊: Physics performance monitoring for 1a4642b

Full contents

physmon summary

benjaminhuth
benjaminhuth previously approved these changes Jun 13, 2024
View reviewed changes
Copy link
Member

@benjaminhuth benjaminhuth left a comment

Choose a reason for hiding this comment

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

If its green, it seems to work. Only the comments in the affected section may need to be rethought...

Core/include/Acts/Propagator/Navigator.hpp Outdated Show resolved Hide resolved
benjaminhuth
benjaminhuth reviewed Jun 13, 2024
View reviewed changes
Copy link
Member

@benjaminhuth benjaminhuth left a comment

Choose a reason for hiding this comment

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

A wait but there are some physmon differences...

@andiwand
Copy link
Contributor Author

A wait but there are some physmon differences...

I suspect that these are coming from the simulation and the low stats in ttbar. I can try to validate this with higher stats or look at a case where it changes

@github-actions github-actions bot added the Stale label Jul 13, 2024
@acts-policybot acts-policybot bot dismissed benjaminhuth’s stale review July 23, 2024 08:40

Invalidated by push of 9416724

@andiwand andiwand added 🚧 WIP Work-in-progress and removed Stale labels Jul 23, 2024
@andiwand andiwand requested a review from noemina July 24, 2024 11:33
@noemina
Copy link
Contributor

noemina commented Jul 24, 2024

If the CI passes, and we understand the differences in the output, then it is fine.

@andiwand andiwand added the 🛑 blocked This item is blocked by another item label Jul 24, 2024
@andiwand
Copy link
Contributor Author

@noemina I investigated the propagation errors a bit and I think I have a solution with the 3 PRs in combination

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Jul 26, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@andiwand andiwand removed the 🚧 WIP Work-in-progress label Aug 6, 2024
@andiwand andiwand mentioned this pull request Aug 19, 2024
Merged
@andiwand
Copy link
Contributor Author

closing in favor of #3437

@andiwand andiwand closed this Aug 20, 2024
@andiwand andiwand deleted the remove-navigator-forceIntersectBoundaries branch August 20, 2024 18:32
@paulgessinger paulgessinger modified the milestones: next, v36.2.0 Aug 26, 2024
kodiakhq bot pushed a commit that referenced this pull request Aug 26, 2024
@andiwand
feat: Renavigation for Gen1 (#3437)
cda0b99 
In case we loose the boundary we have to renavigate. For this purpose I introduce a loop in the `preStep` call where the first iteration has the original behavior and the second one is used in case of renavigation.

Incorporates #3238 to avoid double handling of the renavigation.

blocked by
- #3521
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjaminhuth benjaminhuth benjaminhuth left review comments

@noemina noemina noemina left review comments

Assignees
No one assigned
Labels
Component - Core Affects the Core module 🛑 blocked This item is blocked by another item
Projects
None yet
Milestone
v36.2.0
Development

Successfully merging this pull request may close these issues.
4 participants
@andiwand @noemina @benjaminhuth @paulgessinger