-
Notifications
You must be signed in to change notification settings - Fork 357
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
Correct errors in slicing of node collections #3132
Conversation
…me; first version passing all tests
…ad of filtering according to slicing in a second step.
This reverts commit 33fa658.
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.
Thank you for this contribution 👍. Looks good to me.
8b3578a
to
2619c3f
Compare
7123538
to
abeb018
Compare
…ithub Linux runners due to performance issues; the tests are run under macOS
abeb018
to
e0b5ed4
Compare
…thub now is macOS 14 and only 3.10 and later are available
@gtrensch @jstapmanns Ping! |
The author of the original model with which the problem was originally discovered has reported that the full model now works perfectly! |
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.
Thank you, @heplesser, for implementing these slicing algorithms. In general, I'm pretty confident they do what they are supposed to do, but I still have a question about the example given in the description (which I find very helpful to understand the implementation).
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.
Thank you for this contribution 👍. Approved!
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.
Thorough and elegant implementation 👍
This PR fixes #3108 by providing correct
NodeCollection::{rank,thread}_local_begin(()
implementations for composite node collections.To do:
slice_positions_if_sliced_nc()
code to more suitable locationI removed the ConnBuilder refactoring (see #3106) from this PR to reduce complexity and ensure we can integrate this bug-fixing PR asap. The refactoring can then following in a later PR.