-
Notifications
You must be signed in to change notification settings - Fork 48
Population parameters #1653
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
Open
Christian-B
wants to merge
9
commits into
master
Choose a base branch
from
pop_params
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Population parameters #1653
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
fc517e4
Typed semantic sugar for an additional_parameter
Christian-B 0693edd
remove star in params
Christian-B b376025
changes to please mypy
Christian-B dcf4ba6
flake8
Christian-B d6aa796
Document likely meaning
Christian-B 27e9963
spelling
Christian-B fb556cc
improve docs
Christian-B 6f6fced
Merge branch 'master' into pop_params
Christian-B b682963
Merge branch 'master' into pop_params
Christian-B File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,8 @@ | |
| from spinn_utilities.logger_utils import warn_once | ||
| from spinn_utilities.overrides import overrides | ||
|
|
||
| from pacman.model.partitioner_splitters import AbstractSplitterCommon | ||
|
|
||
| from spinn_front_end_common.utilities.exceptions import ConfigurationException | ||
|
|
||
| from spynnaker.pyNN.data import SpynnakerDataView | ||
|
|
@@ -84,6 +86,15 @@ def __init__( | |
| structure: Optional[BaseStructure] = None, | ||
| initial_values: Optional[Dict[str, float]] = None, | ||
| label: Optional[str] = None, | ||
| *, max_rate: Optional[float] = None, | ||
| n_colour_bits: Optional[int] = None, | ||
| n_synapse_cores: Optional[int] = None, | ||
| neurons_per_core: Optional[Union[int, Tuple[int, ...]]] = None, | ||
| port: Optional[int] = None, | ||
| reserve_reverse_ip_tag: Optional[bool] = None, | ||
| seed: Optional[int] = None, | ||
| splitter: Optional[AbstractSplitterCommon] = None, | ||
| virtual_key: Optional[int] = None, | ||
| additional_parameters: Optional[_ParamDict] = None, | ||
| **additional_kwargs: _ParamDict): | ||
| """ | ||
|
|
@@ -95,17 +106,72 @@ def __init__( | |
| :param structure: | ||
| :param initial_values: Initial values of state variables | ||
| :param label: A label for the population | ||
| :param max_rate: | ||
| Model.create_vertex parameter. | ||
| Likely: | ||
| The maximum number of spikes for any neuron at any timestamp | ||
| :param n_colour_bits: | ||
| Model.create_vertex parameter. | ||
| Likely: The number of bits to use for colour | ||
| :param n_synapse_cores: | ||
| Model.create_vertex parameter. | ||
| Likely: The number of synapse cores; 0 to force combined cores, | ||
| or None to allow the system to choose | ||
| :param neurons_per_core: | ||
| Model.create_vertex parameter. | ||
| Likely: A ceiling on | ||
| the number of neurons that can be placed on a single core. | ||
| :param port: | ||
| Model.create_vertex parameter. | ||
| Likely: The live input port the vertex receives spikes on | ||
| :param reserve_reverse_ip_tag: | ||
| Model.create_vertex parameter. | ||
| Likely: Extra flag for input without a reserved port | ||
| :param seed: | ||
| Model.create_vertex parameter. | ||
| Likely: The Population seed, | ||
| used to ensure the same random generation on each run. | ||
| :param splitter: | ||
| Model.create_vertex parameter. | ||
| Likely: splitter from application vertices to machine vertices | ||
| :param virtual_key: | ||
| Model.create_vertex parameter. | ||
| Likely: | ||
| The virtual_key of the population to identify the population. | ||
| :param additional_parameters: | ||
| Additional parameters to pass to the vertex creation function. | ||
| Alternative way of entering Model.create_vertex parameter. | ||
| Ignored if also provided as a named parameter. | ||
| :param additional_kwargs: | ||
| A nicer way of allowing additional things | ||
| Additional Model.create_vertex parameters. | ||
| See the Model's create_vertex method for more details. | ||
| create_vertex parameters will be ignored if | ||
| the Model does not accept this parameter, | ||
| or raise an Exception if a Vertex is passed in | ||
|
Comment on lines
+146
to
+149
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually think that we should raise an exception in both cases, especially if this is the only use of these additional args. |
||
| """ | ||
| # Deal with the kwargs! | ||
| additional: _ParamDict = dict() | ||
| if additional_parameters is not None: | ||
| additional.update(additional_parameters) | ||
| if additional_kwargs: | ||
| additional.update(additional_kwargs) | ||
| if max_rate is not None: | ||
| additional['max_rate'] = max_rate | ||
| if n_colour_bits is not None: | ||
| additional['n_colour_bits'] = n_colour_bits | ||
| if n_synapse_cores is not None: | ||
| additional['n_synapse_cores'] = n_synapse_cores | ||
| if neurons_per_core is not None: | ||
| additional['neurons_per_core'] = neurons_per_core | ||
| if port is not None: | ||
| additional['port'] = port | ||
| if reserve_reverse_ip_tag is not None: | ||
| additional['reserve_reverse_ip_tag'] = reserve_reverse_ip_tag | ||
| if seed is not None: | ||
| additional['seed'] = seed | ||
| if splitter is not None: | ||
| additional['splitter'] = splitter | ||
| if virtual_key is not None: | ||
| additional['virtual_key'] = virtual_key | ||
|
|
||
| # build our initial objects | ||
| self.__celltype: AbstractPyNNModel | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this unfortunately adds to the confusion; adding these parameters implies all models accept them but in reality they don't. I would prefer that this is left to the model itself to document therefore.
A note here is that PyNN itself doesn't actually accept additional parameters, so what we do with this is our own; we could, in fact, force that all parameters are something that the vertex accepts.
Additional note is that these are not the same as the cell_params (which are, in fact, deprecated). The cell_params are arguments passed when creating the model, the additional_args are arguments passed when creating the vertex, and model is also one of those arguments. In our case, we accept a vertex in place of a model, so this could be resolved that way instead, but it would then start also to look odd when working with standard models, as you would then have something like p.Population(n_neurons, p.PopulationVertex(..., model=p.IF_curr_exp(...))).