Skip to content

Update built-in timer documentation #3490

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

jessica-mitchell
Copy link
Contributor

@jessica-mitchell jessica-mitchell commented Apr 30, 2025

This PR adds a diagram describing a simplified simulation run focusing on timers.
The diagram is generated via a script that converts YAML into a dot diagram. Thanks to @JoseJVS for providing the original version.
In addition, some links are added to try to connect various parts of the documentation together with respect to perforance, benchmarking, and timers.

https://nest-simulator--3490.org.readthedocs.build/en/3490/nest_behavior/built-in_timers.html#built-in-timers

@jessica-mitchell jessica-mitchell added T: Enhancement New functionality, model or documentation S: Normal Handle this with default priority I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Apr 30, 2025
@github-project-automation github-project-automation bot moved this to In progress in Documentation Apr 30, 2025
Copy link
Contributor

@mlober mlober left a 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, it already looks really good. I had a look at the code again and have a few small improvements:

  • diagram: change "Repeat once if buffer too small" to "Repeat once if not all spikes transmitted".
  • diagram: block "if buffer can shrink" must be outside of "Repeat once if not all spikes transmitted".
  • diagram: include another block after time_communicate_spike_data-block that is the opposite of "if buffer can shrink": "if buffer too small -> grow MPI buffer size".
  • OMP parallel and OMP master with same capitalization
  • question to @JanVogelsang : shouldn't the OMP parallel end before OMP master?

@github-project-automation github-project-automation bot moved this from In progress to Review in Documentation Jun 25, 2025
@jessica-mitchell jessica-mitchell requested a review from mlober June 26, 2025 08:08
@JanVogelsang
Copy link
Contributor

JanVogelsang commented Jun 26, 2025

How about making the timer block backgrounds nest-orange instead of white?

shouldn't the OMP parallel end before OMP master?

No that's correct, OMP master sections only make sense within parallel sections, outside of them master is the only thread running anyway

@JanVogelsang
Copy link
Contributor

I'm not sure if "Repeat once if not all spikes transmitted" is actually better than "Repeat once if buffer too small". That makes it sound as if in the first round, the communicated spikes are already used and only the remaining spikes are communicated in the second round.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

3 participants