Provide SP expressions as keys to the SPA associative memory #982

ikajic · 2016-03-14T20:37:52Z

I needed this scenario for my model, so I decided to submit a PR in case it might be of interest for others. Currently, only single keys can be passed as the input and output keys to the associative memory module. This PR slightly changes the existing code so that it also supports expressions in both input or output keys. Then something like this can be done:

import nengo
from nengo import spa

with spa.SPA() as model:
    D = 16

    voc1 = spa.Vocabulary(D)
    voc2 = spa.Vocabulary(D)

    voc1.parse('A+B+C')
    voc2.parse('D+E+F')

    in_keys = ['A', 'B+C']
    out_keys = ['D-E', '0.4*F']

    model.am = spa.AssociativeMemory(input_vocab=voc1,
                                     output_vocab=voc2,
                                     input_keys=in_keys,
                                     output_keys=out_keys
                                    )
    model.inp = spa.State(D, vocab=voc1)
    nengo.Connection(model.inp.output, model.am.input)

jgosmann · 2016-03-14T20:47:16Z

Seem like it could be useful. Can you add unit tests for this?

jgosmann · 2016-03-14T20:48:30Z

nengo/spa/assoc_mem.py


        # If output vocabulary is not specified, use input vocabulary
        # (i.e autoassociative memory)
        if output_vocab is None:
            output_vocab = input_vocab
            output_vectors = input_vectors
+            output_keys = input_keys


This line seems unecessary?

Don't think so...

It only gets used in the else block which initializes it itself if necessary.

And it wasn't there before ..

Yeah, looking at it now I think that line shouldn't be there. Unless @xchoo wants to share a bit more of his thoughts :)

I added it because I thought keys would be used as class attributes/passed to the network.AssociativeMemory, but that doesn't seem to be the case. It seems like those keys are only used to create input and output vectors, if I'm not mistaking...

pblouw · 2016-03-14T20:48:56Z

+1 for this PR - I ran into a similar scenario when I put together my Cogsci submission a while ago.

jgosmann · 2016-03-14T20:53:43Z

Nit-pick: We prefer commit messages to be in the imperative.

ikajic · 2016-03-15T14:40:37Z

Would this be the right place to add tests: https://github.com/nengo/nengo/blob/master/nengo/spa/tests/test_assoc_mem.py?

jgosmann · 2016-03-15T14:57:02Z

Exactly that place. 🎯

ikajic · 2016-03-23T21:46:01Z

I've added a few tests but I would like to discuss how useful they are since the associative memory seems to have a lot of noise when doing this sort of mapping. So I allowed a pretty big error margin -- does this make sense at all? @xchoo @jgosmann

Also, did I just mess up stuff in this branch by merging with the master? 😕

jgosmann · 2016-03-23T22:35:10Z

It's the right idea, but the error margin is too large. The proper fix would probably be to improve the associative memory to produce a more precise output instead of overshooting, but not sure how easily that can be achieved.

We could also say that we don't support exact outputs like 0.3 * A (do you need those?) and thus not test for those. We could still support expressions like 0.5 * A + 0.7 * B or A * POS1 and compare the relative strengths of the vectors.

There are a few assertions in that I don't understand, but I mostly skimmed it so far.

Also, did I just mess up stuff in this branch by merging with the master?

Yes (but it's fixable). Never do merges in the Nengo development model, always to a rebase instead. (It might also make sense to configure git pull to do rebase instead of merge, but that would not have helped here.)

xchoo · 2016-03-23T23:30:41Z

Hmmmm. I think we might want to document that the associative memory works by computing the dot product between the input vector and the output vector (i.e. it is expecting the input vector magnitudes to be nominally 1).

There might be some confusion if this is not stated, because a user might put in, as an input sp phrase, something like 0.5*A, and expect the output of the associative memory to be maximal when the input vector is 0.5*A. In reality though, the output would be 0.25*A.

xchoo · 2016-03-24T00:34:32Z

nengo/spa/tests/test_assoc_mem.py

+        vec_sim = sim.data[prob]
+
+        final_vecA = vec_sim[100:490].mean(axis=0)
+        simA = np.dot(voc2.vectors, final_vecA)


I think in this case, we want to use the vector norm of the vector difference (or the cosine angle) as a comparison here. The dot product is not accurate when the input vectors do not have a magnitude of 1 (-C+D and 0.3*E are not guaranteed to have a magnitude of 1).

ikajic · 2016-03-24T00:54:55Z

[...] exact outputs like 0.3 * A (do you need those?) [..] and compare
the relative strengths of the vectors.

What do you mean by "compare the relative strengths of the vectors"? I
actually had a case where I needed a mapping such as X->0.3*Y, but an
approximation/noisy version was fine for me.

On 23 March 2016 at 15:35, Jan Gosmann notifications@github.com wrote:

It's the right idea, but the error margin is too large. The proper fix
would probably be to improve the associative memory to produce a more
precise output instead of overshooting, but not sure how easily that can be
achieved.

We could also say that we don't support exact outputs like 0.3 * A (do
you need those?) and thus not test for those. We could still support
expressions like 0.5 * A + 0.7 * B or A * POS1 and compare the relative
strengths of the vectors.

There are a few assertions in that I don't understand, but I mostly
skimmed it so far.

Also, did I just mess up stuff in this branch by merging with the master?

Yes (but it's fixable). Never do merges in the Nengo development model,
always to a rebase instead. (It might also make sense to configure git
pull to do rebase instead of merge, but that would not have helped here.)

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#982 (comment)

jgosmann · 2016-03-24T00:59:01Z

What do you mean by "compare the relative strengths of the vectors"?

Normalizing the vectors to length 1 for comparison (i.e. computing the cosine angle; cp. @xchoo's comment).

ikajic · 2016-03-24T18:43:28Z

I've updated tests to use cos similarity and simplified them by presenting just one input. What the current tests are doing is:

testing whether a SP expression can be provided in the input keys
testing whether a SP expression can be provided in the output keys
testing for the right output values with the 0.2 error margin

I agree with Xuan that it might not be clear what are expected outputs, so I think documenting how AM works would fit well in the docstring for the AM module (which is currently a one-liner and would contribute to closing #885), but I think it'd make sense to make another PR for documentation.

jgosmann · 2016-03-24T19:09:55Z

testing whether a SP expression can be provided in the input keys

What would be the use case of providing an expression like 0.9 * A (vs. just A)? It is not clear to me how this would change the operation of the AM and unit tests are also to some degree a usage example. So I might go with an expression like A + B or A * B where I clearly can see a use case.

You're relying on the order of the pointers in the vocab. It would be nicer (for readability) to access them by name instead. For selecting the probe values sim.trange() > 0.15 might be more expressive (it doesn't require me to recall what dt is).

jgosmann · 2016-03-24T21:25:46Z

It might also make sense to configure git pull to do rebase instead of merge, but that would not have helped here.

I guess it would have helped because there is now an additional merge commit which looks exactly like it was produced by a git pull doing a merge. To change that setting add

[pull]
    rebase = true

to your ~/.gitconfig. I'll see if I can get this branch fixed.

jgosmann · 2016-03-24T21:34:30Z

That was pretty easy to fix actually. Just a git rebase -i master (and a git rebase --skip at one point) did it. Note that I had to force push this branch. To get your local branch in sync don't do a git pull, but the following:

git fetch
git checkout spaassocmem-keys-as-expr (if not already on that branch)
git reset --hard origin/spaassocmem-keys-as-expr (WARNING: You lose all local changes and commits on this branch. If you have any local changes/commits you want to keep the process is slightly more complicated. In case you already lost them, because you read this too late, they can still be recovered.)

xchoo · 2016-03-24T22:03:35Z

I've updated the tests. 😄

ikajic · 2016-03-25T16:29:48Z

Thanks for the update @xchoo, looks good to me! Btw. Is there a way to get plots when only running these tests? This line: python -m py.test test_assoc_mem.py --plot doesn't work for me.

Regarding use-cases: I had an example where I needed keys such as 0.9*A+0.3*B+0.1*C in the input. I had examples where mappings to the output keys were depending on the scaling for A, B and C.

tbekolay · 2016-03-25T16:35:11Z

To get plots, the command line option is --plots (note the plural), and the plots get saved to a directory like nengo.simulator.plots in whatever directory you ran py.test from.

jgosmann · 2016-03-25T16:35:21Z

Is there a way to get plots when only running these tests? This line: python -m py.test test_assoc_mem.py --plot doesn't work for me.

It will save the plots in nengo.simulator.plots.

I had an example where I needed keys such as 0.9_A+0.3_B+0.1*C in the input. I had examples where mappings to the output keys were depending on the scaling for A, B and C.

Does this only depend on the relative scaling of A, B, and C? Or is the actual length of the vector important?

ikajic · 2016-03-25T16:37:03Z

Does this only depend on the relative scaling of A, B, and C?

Yeah, it was just the scaling.

jgosmann · 2016-03-25T16:40:44Z

nengo/spa/tests/test_assoc_mem.py

+
+    # Specify t ranges
+    t = sim.trange()
+    t_item1 = (t > 0.075) & (t < 0.1)


I didn't know that you can use the & operator; I always used np.logical_and. This is so much more convenient! :)

jgosmann · 2016-03-25T16:53:01Z

Yeah, it was just the scaling.

In that case I would argue that it is different from 0.9 * A.

Made a few comments in the code, but apart from that it LGTM. 🍰

pblouw · 2016-06-15T21:07:35Z

It would be great to get this into master :)

tcstewar · 2016-06-16T06:29:26Z

It would be great to get this into master :)

I also ran into a situation today where this would have been handy.... So I'm kicking myself for not reviewing this earlier....

Seanny123 · 2016-06-16T13:29:24Z

LGTM.

jgosmann · 2016-06-16T13:35:08Z

This should get a changelog entry.

jgosmann · 2016-06-16T13:40:13Z

I'll add the changelog entry and clean up the history.

Seanny123 · 2016-06-16T13:47:45Z

Looks even better to me.

This allows to use Semantic Pointer expressions like '0.5*A + 0.3*B' as input_keys and output_keys to the AssociativeMemory. Before it was only possible to give specific Semantic Pointers like 'A'. Co-authored by: Xuan Choo <xchoo.mainframe@gmail.com>

Add changelog entry.

jgosmann · 2016-06-16T13:55:10Z

I cleaned the history up, added the changelog entry, and added another fixup commit. If I get an ok on this, I can merge this.

Seanny123 · 2016-06-16T13:58:20Z

I am now utterly convinced that this is the best this commit can possibly be.

jgosmann reviewed Mar 14, 2016
View reviewed changes

Seanny123 added the work in progress label Mar 15, 2016

xchoo reviewed Mar 24, 2016
View reviewed changes

jgosmann force-pushed the spaassocmem-keys-as-expr branch from 56e34ea to 3116088 Compare March 24, 2016 21:29

xchoo added needs review and removed work in progress labels Mar 24, 2016

jgosmann reviewed Mar 25, 2016
View reviewed changes

Seanny123 added the needs changes label Apr 22, 2016

Seanny123 removed the needs review label Apr 22, 2016

jgosmann self-assigned this Jun 15, 2016

jgosmann force-pushed the spaassocmem-keys-as-expr branch from 71ba93d to a2c8cab Compare June 15, 2016 21:13

jgosmann removed their assignment Jun 15, 2016

Seanny123 added needs review and removed needs changes labels Jun 16, 2016

jgosmann self-assigned this Jun 16, 2016

jgosmann added this to the 2.1.1 release milestone Jun 16, 2016

jgosmann force-pushed the spaassocmem-keys-as-expr branch from 044347b to a483ebb Compare June 16, 2016 13:50

ikajic and others added 3 commits June 16, 2016 09:53

squash! Allow SP expressions in Associative memory keys.

eb5e3a2

Add changelog entry.

fixup! Allow SP expressions in Associative memory keys.

ea15d64

jgosmann force-pushed the spaassocmem-keys-as-expr branch from 3ae95a3 to ea15d64 Compare June 16, 2016 13:53

jgosmann removed their assignment Jun 16, 2016

Seanny123 added reviewed and removed needs review labels Jun 16, 2016

jgosmann self-assigned this Jun 16, 2016

jgosmann merged commit ea15d64 into master Jun 16, 2016

jgosmann removed their assignment Jun 16, 2016

Provide SP expressions as keys to the SPA associative memory #982

Provide SP expressions as keys to the SPA associative memory #982

Conversation

ikajic commented Mar 14, 2016

jgosmann commented Mar 14, 2016

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

pblouw commented Mar 14, 2016

jgosmann commented Mar 14, 2016

ikajic commented Mar 15, 2016

jgosmann commented Mar 15, 2016

ikajic commented Mar 23, 2016

jgosmann commented Mar 23, 2016

xchoo commented Mar 23, 2016

Choose a reason for hiding this comment

ikajic commented Mar 24, 2016

jgosmann commented Mar 24, 2016

ikajic commented Mar 24, 2016

jgosmann commented Mar 24, 2016

jgosmann commented Mar 24, 2016

jgosmann commented Mar 24, 2016

xchoo commented Mar 24, 2016

ikajic commented Mar 25, 2016

tbekolay commented Mar 25, 2016

jgosmann commented Mar 25, 2016

ikajic commented Mar 25, 2016

Choose a reason for hiding this comment

jgosmann commented Mar 25, 2016

pblouw commented Jun 15, 2016

tcstewar commented Jun 16, 2016

Seanny123 commented Jun 16, 2016

jgosmann commented Jun 16, 2016 • edited Loading

jgosmann commented Jun 16, 2016

Seanny123 commented Jun 16, 2016

jgosmann commented Jun 16, 2016

Seanny123 commented Jun 16, 2016

jgosmann commented Jun 16, 2016 •

edited

Loading