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

Add native support for networks represented in SONATA format #2595

Merged
merged 237 commits into from
May 15, 2023

Conversation

nicolossus
Copy link
Member

This PR adds support for building and simulating networks of point neurons represented by the SONATA format.

@nicolossus
Copy link
Member Author

I have merged with the current master and done the necessary updates.

@heplesser and @jessica-mitchell The documentation on configuring NEST for SONATA in nest_sonata_guide.rst is updated and I also added some context for the HDF5 option in cmake_options.rst. Do you think the documentation looks fine for now?

@jougs I have adapted the build matrix to also include HDF5 for the full NEST build. Can you check that the changes are okay?

Copy link
Contributor

@jessica-mitchell jessica-mitchell left a comment

Choose a reason for hiding this comment

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

@nicolossus Looks good, just noticed that the link to the example is broken because the path is incorrect (see comments), also you need to add a README to the pynest/example/sonata_example/ page for sphinx-gallery to generate output from subdirectories.

doc/htmldoc/nest_sonata/nest_sonata_guide.rst Outdated Show resolved Hide resolved
doc/htmldoc/examples/index.rst Outdated Show resolved Hide resolved
doc/htmldoc/nest_sonata/nest_sonata_guide.rst Outdated Show resolved Hide resolved
nicolossus and others added 2 commits May 4, 2023 13:34
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
@nicolossus
Copy link
Member Author

@jessica-mitchell Thanks, fixed your comments.

Copy link
Contributor

@jessica-mitchell jessica-mitchell left a comment

Choose a reason for hiding this comment

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

Great, looks good!

@jougs jougs requested a review from heplesser May 4, 2023 12:40
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.

@nicolossus This looks very fine, I just have a few details, see inline.

doc/htmldoc/installation/cmake_options.rst Outdated Show resolved Hide resolved
doc/htmldoc/nest_sonata/nest_sonata_guide.rst Outdated Show resolved Hide resolved
pynest/nest/lib/hl_api_sonata.py Outdated Show resolved Hide resolved
pynest/nest/lib/hl_api_sonata.py Show resolved Hide resolved
nestkernel/sonata_connector.cpp Outdated Show resolved Hide resolved
@nicolossus
Copy link
Member Author

@heplesser I have now addressed all your comments and suggestions. All links should now be to the most sensible destination, the consistency checks in sonata_connector.cpp have been refactored and I added some more comments on the logic in hl_api_sonata.py.

@nicolossus nicolossus requested a review from heplesser May 9, 2023 14:28
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.

@nicolossus Thanks a lot, looks good to me and the failing on macOS seems to be a runner problem, so I will approve now, assuming that the test will pass on re-run.

@heplesser heplesser removed the request for review from jougs May 11, 2023 07:15
Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

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

Just three minor formal changes. Otherwise this looks good to me.

doc/htmldoc/nest_sonata/nest_sonata_guide.rst Outdated Show resolved Hide resolved
doc/htmldoc/nest_sonata/nest_sonata_guide.rst Outdated Show resolved Hide resolved
doc/htmldoc/nest_sonata/nest_sonata_guide.rst Outdated Show resolved Hide resolved
@jougs jougs merged commit a3623b0 into nest:master May 15, 2023
@nicolossus nicolossus deleted the nestsonata branch May 15, 2023 20:23
@terhorstd terhorstd changed the title Add native support for networks represented by the SONATA format Add native support for networks represented in SONATA format Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: User Interface Users may need to change their code due to changes in function calls S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

10 participants