-
Notifications
You must be signed in to change notification settings - Fork 157
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
refactor: Refactor loop protection #2535
refactor: Refactor loop protection #2535
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, we lose the direction sign, but that's intended because we don't want to apply this when stepping back (e.g. overstepping), and when we're actually going backward, the path length is also positive?
Codecov Report
@@ Coverage Diff @@
## main #2535 +/- ##
==========================================
- Coverage 49.77% 49.76% -0.01%
==========================================
Files 466 466
Lines 26322 26326 +4
Branches 12095 12097 +2
==========================================
Hits 13101 13101
- Misses 4621 4624 +3
- Partials 8600 8601 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
These changes make the path limit more sound in the CKF. Before we did not respect the path limit during target surface propagation which can result in propagation step overflow for loopers. At the same time I tried to improve the error handling by not aborting the whole propagation in case of an error but keep going on until we handled all branches. This is not fully communicated downstream yet but will still result in error logs we can catch. Pulled out of #2518 bocked by - #2535
The loop protection should not be direction dependent anymore. The limit should be set even if it is higher.
Pulled this out of #2518