-
Notifications
You must be signed in to change notification settings - Fork 370
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
Separate event transmission and event delivery for better performance in high-thread situations #2617
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
…spike_data; use max spike-data buffer size
…ther_spike_data Ensure thread-local memory allocation
…om:suku248/nest-simulator into test_single_threading_in_gather_spike_data
…gle_threading_in_gather_spike_data
…m:suku248/nest-simulator into single_batchwise
…single_threading_in_gather_spike_data Conflicts: nestkernel/event_delivery_manager.cpp
Pull request automatically marked stale! |
github-actions
bot
added
the
stale
Automatic marker for inactivity, please have another look here
label
Jun 10, 2023
…ert to parametrize
Closing this PR. It is superseeded by #2926. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
I: External API
Developers of extensions or other language bindings may need to adapt their code
S: High
Should be handled next
stale
Automatic marker for inactivity, please have another look here
T: Enhancement
New functionality, model or documentation
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 a draft PR to integrate the revision of event transmission from the "5G" approach to close to what we had in "4G" (until NEST 2.14). Briefly, the events are now gathered at the end of each update loop and MPI-communicated, and then delivered at the beginning of the next update loop.
At present, all kinds of events are supported again (SpikeEvent on and off grid, SecondaryEvent), but test_mini_brunel_ps.sli fails if NEST is compiled with GCC 12. When compiling with Clang 13, the test passes. This indicates most likely some incorrect memory access. Failure is deterministic with spike times depending on the mpi-thread split. Results are correct for a single MPI process (four threads).
To get more insight into where things go wrong, I attempted to build NEST with
-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -fsanitize=address
As long as the
*DEBUG*
options were in place, g++ emits error messages concerning overridden deleted functions in theSecondaryEvent
class hierarchy, which left me entirely confused, so I dropped them. With just-fsanitize=address
, NEST builds but segfaults on start with a stack trace indicating that the problem occurs even before NEST itself starts. @med-ayssar and @JanVogelsang, could you two take a look at this?Once this problem is solved, a number of this remain. I will comment on them later in this issue.
I consciously add many reviewers just to make sure you are in the loop.