pyrates-neuroscience / PyRates

Open-source, graph-based Python code generator and analysis toolbox for dynamical systems (pre-implemented and custom models). Most pre-implemented models belong to the family of neural population models.
https://pyrates.readthedocs.io/en/latest/
GNU General Public License v3.0
73 stars 8 forks source link

Allow multiple source variables in edges #14

Closed dafrose closed 4 years ago

dafrose commented 4 years ago

The goal is to enable dynamics in edges that need more than one input variable from the same node but do not make sense to move into nodes for logical or performance reasons. This feature was originally requested by @Richert .

Suggested YAML syntax:

MyCircuit: 
  ...
  edges:
    # single source variable as before
    # [source, target, edge_template, values]
    - [source_node/op/var, target_node/op/var, MyEdge, {weight: 1, ...}]
    # multiple source variables need to map source variables to edge variables
    # Option 1: keep syntax and repeat source node
    - [{source_node/op1/var1: edge_input_var1, 
         source_node/op2/var2: edge_input_var2, 
         ...}, 
       target_node/op/var, MyMultiEdge, {<variables>}]
    # Option 2: put source node in front and only reference ops/vars
    - [source_node: {op1/var1: edge_input_var1,
                               op2/var2: edge_input_var2, 
                               ...}, 
       target_node/op/var, MyMultiEdge, {<variables>}]

The previous syntax does not need to be changed, because single input variables can be uniquely identified. However, multiple input variables need to be mapped directly to source variables which complicates the syntax. Keeping the mapping logic of source -> target I prefer mapping <source_var>: <edge_var> over the alternative.

Option 1 is consistent with the previous syntax but repeating the source node is (1) more verbose which may be cumbersome and (2) suggests that more than one source node may exist which breaks the template. Option 2 makes it clear that only one source node exists but introduces a new syntax that is inconsistent with the single-variable case and the target variable syntax.

I prefer Option 2 for its clarity and comparative brevity.

The Python API should be updated the same way as the YAML syntax.

Note that variables used as input variables in the edge definition of a circuit also need to be defined as input variables in operators in the edge template. Furthermore the number of variables flagged as input in an edge template should always be the same as the assigned source variables in order to be consistent with current syntax. However, we could implement the system in such a way that a circuit can update variable definitions so the same template can be used with a different number of inputs as required.

Code changes that need to be made:

Note: The entire frontend code is surprisingly ignorant as to the content of source/target definitions and number of inputs.

dafrose commented 4 years ago

I just noticed that input variables should always be unique across an edge (or node for that matter). So we only need to map {source_op/source_var: input_var} instead of {source_op/source_var: edge_op/edge_var}. Adapted original post accordingly.

dafrose commented 4 years ago

experimental support for multiple source variables is added with f8f31cf4dc9403268493797c7777200af8e195f9 . I opted for Option 2 with dictionaries for multiple variables. Needs to be tested.

Note: The inputs attribute of an EdgeIR currently returns a dictionary of form {"input_var": [op/input_var, ...]}. But during vectorization only the first reference in list is used per variable. I am unsure whether this works in general or only with the current examples.