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

Nest developer space: Description for conversion to 5g #964

Merged
merged 6 commits into from
Jun 14, 2018

Conversation

steffengraber
Copy link
Contributor

5g introduces a few changes to model code. In order to help users and developers to update their custom models to NEST 2.16 this text describes the necessary changes to neuron models, synapse models and devices.
See jakobj#72

@steffengraber steffengraber changed the title Description for conversion to 5g Nest developer space: Description for conversion to 5g Jun 6, 2018
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.

Nice, I have just small comments.

index.md Outdated
versions to be compatible with the latest release (2.16). In the following we describe
all necessary changes:

* [Updating models from 2.14 or prior to 2.16](model_conversion_5g.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

I like that the details are in a separate file, but I am not sure about the format of the link here: (i) Does a single-item bullet list make sense? (ii) Could the reader be confused because the link text is the same as the section heading? How about replacing the text from "In the following ..." with something like [Please see these instructions for adapting models to NEST 2.16](model_conversion_5g.md)?
@jessica-mitchell What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the format matches quite nicely with the rest of the page (i.e. links are bullet lists), which is good for consistency and making the link stand out. However, I agree that the link text and title might be better if different from each other. What do you think of something like removing the headings "Updating models from ..." and putting both texts and links under a heading "Updating Models for the 5g Kernel" or "Updating Models for Newest Release"?
Note: I am getting a 404 on the page for "updating models from 2.4 or prior to 2.6".

Copy link
Contributor Author

@steffengraber steffengraber Jun 8, 2018

Choose a reason for hiding this comment

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

I followed @jessica-mitchell suggestion and changed it accordingly. Jessica, as you may have noticed, has all internal links point to 404. You would have to explicitly point to the file with the extension '.md', which is a problem for the HTML parser,

index.md Outdated
## Updating models from 2.14 or prior to 2.16

With the introduction of the new connection infrastructure of the [5g kernel](https://www.frontiersin.org/articles/10.3389/fninf.2018.00002/full),
neuron, synapse and device models need to be slightly adapted from prior
Copy link
Contributor

Choose a reason for hiding this comment

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

@jakobj @suku248 Should we make this "rate neuron, synapse, and device models"?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, please replace neuron with rate neuron, also in the introduction in the other file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@heplesser heplesser added ZC: Documentation DO NOT USE THIS LABEL ZP: PR Created DO NOT USE THIS LABEL S: Critical Needs to be addressed immediately 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 Jun 6, 2018
@heplesser heplesser added this to the NEST 2.16 milestone Jun 6, 2018
index.md Outdated
@@ -72,16 +72,16 @@ can either be static or implement synaptic plasticity rules such as
* [Synapses in NEST: An overview](synapses_overview)
* [Developing synapse models](synapse_models)

## Updating models from 2.14 or prior to 2.16
## Updating Models for the 5g Kernel
Copy link
Contributor

Choose a reason for hiding this comment

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

@steffengraber "5g kernel" is a term we are using only internally, so it may confuse users, so we should use 2.14 and 2.16 here, and similarly below for "4g". To avoid having the same line twice, once as heading and once as bullet point, I'd suggest to change the text in the bullet point to "How to update models from 2.14 or prior to 2.16" and corresponding for 4g. Sorry I didn't make an explicit suggestion earlier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, now it is:

## Updating models to 2.16
* [How to update models from NEST 2.14 or prior to 2.16](model_conversion_5g)

## Updating models to 2.6 or later
* [How to update models from NEST 2.4 or prior to 2.6 or later](model_conversion_3g_4g)

@heplesser
Copy link
Contributor

@suku248 Could you double-check that the content of these instructions is okay?

Copy link
Contributor

@suku248 suku248 left a comment

Choose a reason for hiding this comment

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

I double-checked and didn't find anything that needs to be added to the instructions

@heplesser heplesser merged commit 08844a5 into nest:gh-pages Jun 14, 2018
@steffengraber steffengraber deleted the gh-pages-model-conversion-5g branch September 6, 2023 05:23
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: Critical Needs to be addressed immediately T: Maintenance Work to keep up the quality of the code and 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.

None yet

5 participants