Add support for different binding operations #198

jgosmann · 2018-07-13T21:10:27Z

Motivation and context:
This adds the possibility to change the binding (and superposition) operation used by Nengo SPA. It also implements vector-derived transformation binding (VTB) as one alternate binding method.

The specific choice of a superpositiion operation and a binding operation is called an algebra in this PR (as it roughly fits the mathematical definition of an algebra or algebraic structure). Such in algebra is implemented by a class providing pure math implementations of the operations, methods to obtain special elements like the identity, and two methods to provide neural implementations of the two operators.

Each vocabulary (and SemanticPointer which might not have a vocabulary) is assigned such an algbra determining the operations used when operands get combined in the SPA syntax (it defaults to CircularConvolutionAlgebra). Because operands are required to have the same vocabulary, it is always clear what algebra to use, i.e., there is no situation in which the operands might have different algebras (except for SemanticPointer which raises an exception in this case).

Currently there is not way to change the default algebra, but one has to explicitely create a vocabulary with the desired algbra. Not sure whether there should be a way to set this default ...

Closes #69.

Interactions with other PRs:
none

How has this been tested?

Tests have been added, updated, etc.
An algebra fixture has been added that will parametrize a test to run with both implemented algebras.

How long should this take to review?

Lengthy (more than 150 lines changed or changes are complicated)

Where should a reviewer start?

The commits should be self-contained, so that one could go commit by commit. But I revised some things multiple times and the back and forth might not be that helpful. Such it might be better to look at the final diff. There are multiple parts that stand mostly on its own:

Added matrix multiplication network nengo_spa/networks/matrix_multiplication.py.
Added module nengo_spa/modules/superposition.py.
The interface for algebras nengo_spa/algebras/base.py.
The circular convolution algebra nengo_spa/algebras/cconv.py. (Probably best to look at this one first as it uses familiar operations.)
The VTB algebra nengo_spa/algebras/vtb.py with the binding network nengo_spa/networks/vtb.py.
Several smaller changes in nengo_spa/vocab.py and nengo_spa/pointer.py and some other files to enable the use of algebras.
All of this is accompanied with changes in the respective test files and documentation.

Types of changes:

Breaking change (fix or feature that causes existing functionality to change)

Checklist:

I have read the CONTRIBUTING.rst document.
I have updated the documentation accordingly.
I have included a changelog entry.
I have added tests to cover my changes.
I have run the test suite locally and all tests passed.

Still to do:

Extract the VTB network into its own class.
Try to cut down on the increased test duration.

Seanny123 · 2018-07-17T21:58:52Z

nengo_spa/pointer.py

        Vocabulary that the Semantic Pointer is considered to be part of.
+        Mutually exclusive with the *algebra* argument.


Why different formatting for algebra and vocab?

Good catch.

Seanny123 · 2018-07-17T22:00:17Z

nengo_spa/pointer.py

-        type_ = infer_types(self, other)
-        vocab = None if type_ == TAnyVocab else type_.vocab
-        return SemanticPointer(data=self.v - other.evaluate().v, vocab=vocab)
+        return self + (-other)


Why does __add__ need type checking, while __sub__ doesn't?

Because sub re-expresses a - b as a + (-b) thus using the implementation of __add__ and __neg__ which already do type checking.

Seanny123 · 2018-07-18T18:43:39Z

docs/user_guide/spa_intro.rst

@@ -180,4 +180,4 @@ All of the control-like steps (e.g. “compared with”, “inferred”, and rou
 information through the system), are implemented by a biologically plausible
 basal ganglia model. This is one example of the 8 different tasks that Spaun is
 able to perform. Videos for all tasks can be found `here
-<http://nengo.ca/build-a-brain/spaunvideos/>`_.
+<http://www.nengo.ca/build-a-brain/spaunvideos/>`_.


This should probably be changed to https://xchoo.github.io/spaun2.0/videos.html which it redirects to anyways. If I leave tiny nit-picks like this in my review, you can safely assume I'll change them myself once I've reviewed the other files.

Seanny123 · 2018-07-18T19:16:28Z

nengo_spa/algebras/vtb.py

@@ -58,10 +58,22 @@ def invert(cls, v):
        return v.reshape((sub_d, sub_d)).T.flatten()

    @classmethod
-    def get_binding_matrix(cls, v):
+    def get_binding_matrix(cls, v, swap_inputs=False):


I'm not sure I understand the point of swapping inputs as a function argument. Naively, it seems to me that the inputs should just be swapped in the connections and not as a parameter, if the user wants that. Would you mind explaining to me the intent of this?

When using a binding matrix one vector is fixed (the one the matrix) is based on and one varies over time. Thus, there is no way to swap the inputs because there is only on time varying input.

Seanny123 · 2018-07-18T19:57:33Z

I've read through the code once, but I'd like to read it again. It's pretty remarkable how you managed to abstract such a fundamental part of SPA.

Seanny123 · 2018-08-08T21:26:23Z

nengo_spa/algebras/base.py

+
+        Parameters
+        ----------
+        v : n(d,) darray


What is n here?

Seanny123 · 2018-08-08T22:03:39Z

Made some formatting fixes and added a test to make sure the unitary vector doesn't change the length of what it's binded with. This PR LGTM.

Copied from nengo-extras.

Also fix VTB unbinding implementation.

The CircularConvolutionAlgebra essentially implements the same functionality.

jgosmann · 2018-08-20T22:47:09Z

@Seanny123, can you “officially” approve this PR?

jgosmann added work in progress API change labels Jul 13, 2018

jgosmann added this to the 0.6 milestone Jul 13, 2018

jgosmann self-assigned this Jul 13, 2018

jgosmann force-pushed the algebras branch 2 times, most recently from a82d6b0 to 1756bb1 Compare July 17, 2018 19:02

jgosmann removed the work in progress label Jul 17, 2018

jgosmann removed their assignment Jul 17, 2018

jgosmann requested review from Seanny123 and tcstewar July 17, 2018 19:03

jgosmann mentioned this pull request Jul 17, 2018

Naming inconsistencies #199

Closed

Seanny123 reviewed Jul 17, 2018

View reviewed changes

Seanny123 reviewed Jul 18, 2018

View reviewed changes

This was referenced Jul 19, 2018

Different ways to generate Semantic Pointers #201

Merged

Track names of pointers from vocabulary #202

Merged

Seanny123 reviewed Aug 8, 2018

View reviewed changes

nengo_spa/algebras/base.py Outdated

Parameters

----------

v : n(d,) darray

Copy link

Collaborator

Seanny123 Aug 8, 2018

Choose a reason for hiding this comment

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

What is n here?

jgosmann force-pushed the algebras branch from bd442f2 to 34ee5fc Compare August 20, 2018 19:40

jgosmann self-assigned this Aug 20, 2018

jgosmann added 8 commits August 20, 2018 16:54

Extract basic circular convolution algebra.

8b0a036

Define special elements via algebra.

7dbc8ea

Make neural implementations dependent on algebra.

e15fe71

Implement VTB math.

df6b33b

Add matrix multiplication network.

6554d86

Copied from nengo-extras.

Add neural VTB implementation.

77df71e

Allow to swap inputs for binding matrix.

d9d5c4a

Rename invert_left/right to unbind_*.

b00cea8

Also fix VTB unbinding implementation.

jgosmann added 21 commits August 20, 2018 16:54

Sort test plots into directories by algebra.

b09643c

Reduce duration of tests.

6742d67

Extract VTB network.

5447c24

Remove circconv function.

20eb4fb

The CircularConvolutionAlgebra essentially implements the same functionality.

Add __init__.py file to algebras module.

417b27c

Better message for sp_close assertions.

242f4d9

Introduce abstract base class for algebras.

6438c19

Actually use RMSE (not sum) in assert_sp_close.

bb7d0ac

Make algebras singleton classes.

ab89713

Update documentation for algebras.

8408289

Remove unused import.

6e91ee8

Add user documentation for algebras.

dc9cc79

Add more tests to increase coverage.

0ef442d

Exclude conftest.py from coverage statistics.

4952373

Add changelog for algebras implementation.

5a31685

Reduce code duplication.

0041055

Make matrix mult. network adhere to standards.

b0136a1

Fix doc build/check.

c11460e

Fix links in documentation.

a9cd073

Fix cconv make_unitary corner cases.

0bae0db

Improve minor issues in documentation.

176291f

jgosmann force-pushed the algebras branch from 34ee5fc to 176291f Compare August 20, 2018 20:55

Seanny123 approved these changes Aug 21, 2018

View reviewed changes

jgosmann merged commit caa714a into master Aug 21, 2018

jgosmann deleted the algebras branch August 21, 2018 01:49

jgosmann mentioned this pull request Aug 22, 2018

Making vocabulary subsets reinterpret automatically #183

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for different binding operations #198

Add support for different binding operations #198

jgosmann commented Jul 13, 2018 •

edited

Loading

Seanny123 Jul 17, 2018

jgosmann Jul 18, 2018

Seanny123 Jul 17, 2018

jgosmann Jul 18, 2018

Seanny123 Jul 18, 2018

Seanny123 Jul 18, 2018

jgosmann Jul 18, 2018

Seanny123 commented Jul 18, 2018

Seanny123 Aug 8, 2018

Seanny123 commented Aug 8, 2018

jgosmann commented Aug 20, 2018

		Vocabulary that the Semantic Pointer is considered to be part of.
		Mutually exclusive with the algebra argument.

Add support for different binding operations #198

Add support for different binding operations #198

Conversation

jgosmann commented Jul 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Seanny123 commented Jul 18, 2018

Choose a reason for hiding this comment

Seanny123 commented Aug 8, 2018

jgosmann commented Aug 20, 2018

jgosmann commented Jul 13, 2018 •

edited

Loading