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

Improve documentation of rate-based models #1484

Merged
merged 4 commits into from
Apr 1, 2020

Conversation

ddahmen
Copy link
Contributor

@ddahmen ddahmen commented Mar 26, 2020

This pull request is related to issue #1467: Improve documentation for rate-based models.
It comprises the following changes:

  1. Added equation for the rate_transformer to the documentation of the template.
  2. The documentation of all rate neurons was updated by adding the following information:
    • which neurons are available (input noise ipn, output noise opn, rate transformer)
    • how to create them (explicit mentioning of suffixes ipn and opn for input and output noise)
    • preparation for cross-links to templates
    • note on parameters

@ddahmen
Copy link
Contributor Author

ddahmen commented Mar 26, 2020

Suitable reviewers would be @sarakonradi and @jstapmanns

@heplesser
Copy link
Contributor

@ddahmen Could you edit the issue title so it reflects the changes made and mention the issue fixed only in the PR description? We generate release notes from the PR titles, so they should be informative.

@ddahmen ddahmen changed the title Fix issue #1467 Improved documentation of rate-based models Mar 27, 2020
@ddahmen ddahmen changed the title Improved documentation of rate-based models Improve documentation of rate-based models Mar 27, 2020
@terhorstd terhorstd added ZC: Documentation DO NOT USE THIS LABEL I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) ZP: PR Created DO NOT USE THIS LABEL S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation labels Mar 27, 2020
Copy link
Contributor

@sarakonradi sarakonradi left a comment

Choose a reason for hiding this comment

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

@ddahmen Thanks a lot for adding the rate_transformer equation and nest.Create() descriptions! This is great.

I added a few minor comments regarding grammar and punctuation.

models/lin_rate.h Outdated Show resolved Hide resolved
models/lin_rate.h Outdated Show resolved Hide resolved
models/sigmoid_rate.h Outdated Show resolved Hide resolved
Copy link
Contributor

@jstapmanns jstapmanns left a comment

Choose a reason for hiding this comment

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

@ddahmen Thank you for improving the documentation. I just have a small comment on a line that you didn't even touch.

models/gauss_rate.h Outdated Show resolved Hide resolved
@ddahmen
Copy link
Contributor Author

ddahmen commented Mar 31, 2020

Thanks @sarakonradi and @jstapmanns for going through my changes.
I followed your suggestions and updated the documentation accordingly.

Copy link
Contributor

@jstapmanns jstapmanns 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 addressing my comment.

Copy link
Contributor

@sarakonradi sarakonradi left a comment

Choose a reason for hiding this comment

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

@ddahmen Great, thanks for the update.

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 ZC: Documentation DO NOT USE THIS LABEL ZP: PR Created DO NOT USE THIS LABEL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants