Skip to content

Conversation

clinssen
Copy link
Contributor

@clinssen clinssen commented Jan 6, 2025

No description provided.

@clinssen clinssen added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Jan 6, 2025
@terhorstd
Copy link
Contributor

This needs a quick review for correctness. Single reviewer should be fine.

Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

@clinssen Thanks for the PR. Please see my inline comments for some corrections on details.

Comment on lines 290 to 292
* This method is called every ``min_delay`` interval, with each step
* between ``from`` and ``to`` corresponding to one simulation resolution
* (``nest.resolution``) timestep.
Copy link
Contributor

Choose a reason for hiding this comment

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

This also needs to be updated corresponding to the changes above.

* Bring the node from state $t$ to $t+n*dt$, sends SecondaryEvents
* (e.g. GapJunctionEvent) and resets state variables to values at $t$.
* Advance the state of the node forward in time by one ``min_delay``
* interval (see ``update()``); send SecondaryEvents (e.g. GapJunctionEvent)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same points as for update() apply here. Also, can one mark up the "see update()" here in such a way that update() becomes a hyperlink to the documentation for update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jessica-mitchell: could you advise on this? Cheers!

Copy link
Contributor

@jessica-mitchell jessica-mitchell Jun 30, 2025

Choose a reason for hiding this comment

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

We don't have links to C++ functions in Sphinx @clinssen

@clinssen clinssen requested a review from heplesser March 31, 2025 08:23
@github-project-automation github-project-automation bot moved this to In progress in Documentation Jul 25, 2025
@terhorstd terhorstd requested review from otcathatsya and heplesser and removed request for heplesser and otcathatsya August 25, 2025 10:21
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: Maintenance Work to keep up the quality of the code and documentation.
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

4 participants