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.

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

Polish event dispatch #2434

Merged

Conversation

thorstenhater
Copy link
Contributor

@thorstenhater thorstenhater commented Jan 7, 2025

Re-factor event dispatch, making it slightly faster.
Builds on #2433
Closes #2431

ErbB4
ErbB4 previously approved these changes Jan 7, 2025
Reason: STL distributions are platform dependent, thus cause different
results on AClang ./. GCC+Clang
}

// TODO parallelise over streams, however, need a proper testcase/benchmark.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should have an issue for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can make one, it's a very open ended project, though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

while(m) {
cell_gid_type val = dis(gen);
// simple, compiler independent int in range
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is just an example model, I guess it's fine. But there might be a bias introduced by this approach (see here). So adding a warning note as a comment would be good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, fixed.

@thorstenhater thorstenhater requested a review from jlubo March 5, 2025 21:04
@thorstenhater thorstenhater reopened this Mar 7, 2025
@thorstenhater thorstenhater merged commit c4ff08c into arbor-sim:master Mar 7, 2025
53 of 55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Brunel HH Example behaves differently with Apple Clang.
3 participants