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

added brunel network tutorial #2387

Merged
merged 11 commits into from
Aug 27, 2024
Merged

Conversation

ErbB4
Copy link
Collaborator

@ErbB4 ErbB4 commented Aug 19, 2024

added Brunel network tutorial on Arbor

Closes: #1933

@ErbB4 ErbB4 requested a review from thorstenhater August 19, 2024 14:36
Copy link
Contributor

@thorstenhater thorstenhater left a comment

Choose a reason for hiding this comment

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

Finally someone does this. :D

doc/tutorial/brunel.rst Show resolved Hide resolved
doc/tutorial/brunel.rst Show resolved Hide resolved
doc/tutorial/brunel.rst Outdated Show resolved Hide resolved
:lines: 62-81


The Brunel network is randomly sparsely connected with a fixed in-degree regulated by a connection probability (:math:`\epsilon`). We, therefore, define a function to enable random connectivity.
Copy link
Contributor

Choose a reason for hiding this comment

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

A short explanation what sample_subset aims to achieve?

doc/tutorial/brunel.rst Outdated Show resolved Hide resolved
python/example/brunel/analysis.py Outdated Show resolved Hide resolved
python/example/brunel/analysis.py Show resolved Hide resolved
sim.run(tfinal * U.ms, dt * U.ms)


print("This is simulating Brunel network in Arbor!")
Copy link
Contributor

Choose a reason for hiding this comment

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

Needed? Looks like debugging :D

python/example/brunel/parameters.py Outdated Show resolved Hide resolved
python/example/brunel/nest_brunel.py Outdated Show resolved Hide resolved
Copy link
Contributor

@thorstenhater thorstenhater left a comment

Choose a reason for hiding this comment

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

Almost there, a few comments:

  1. The images do not render in the preview build, something is off there. See https://arbor--2387.org.readthedocs.build/en/2387/tutorial/brunel.html#the-full-code
  2. Lint is failing due to the #### marks. I'd suggest removing those.
  3. Lint also doesn't like import *, but here it's ok. I'll find a way to disable it, if you like. Alternatively, you could use a really short name, e.g. import parameters as P.

doc/tutorial/brunel.rst Outdated Show resolved Hide resolved
doc/tutorial/brunel.rst Outdated Show resolved Hide resolved
doc/tutorial/brunel.rst Outdated Show resolved Hide resolved
doc/tutorial/brunel.rst Outdated Show resolved Hide resolved
file.text Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

This one shouldn't be here, right?

.flake8 Outdated
@@ -23,3 +23,7 @@ extend-exclude =
build,
# nah, don't care
spack/package.py
per-file-ignores=
python/example/brunel/analysis.py=F405,F403
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
python/example/brunel/analysis.py=F405,F403
python/example/brunel/analysis.py:F405,F403

and the next two lines.

import numpy as np


#### define network parameters ####
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#### define network parameters ####
# define network parameters

@thorstenhater thorstenhater marked this pull request as ready for review August 21, 2024 13:15
@thorstenhater thorstenhater merged commit a4c3fa6 into arbor-sim:master Aug 27, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Brunel tutorial: example already exists.
2 participants