Connections: replace destroyMinPermSynapses() with plausible decay

[breznak]

• [x] removes Connections.destroyMinPermSynapses() as not being biologically plausible
  • replaced by already existing functionality of decaying synaptic weights for un/mis-used synapses and removing "dead" synapses (too low permanence)
  • adapted test with explanation to show this behavior works
• interestingly, this is not seen to have any effect in c++ Hotgym, while hotgym.py excercises the code (uses different TM params, real-world input data), and results are +- the same or slightly better.
• performance is the same
• TM code is cleaner

TODO:

• [ ] implement synaptic decay (and verify performance impact)
• [ ] test on NAB (to see reasonable accuracy)

Followup to #746

[breznak]

pingity-pong, please :)

[breznak]

don't really understand what this change does.

replaces the "let's remove the weakest synapse to make room for a new one", with

• a segment can fit only so many synapses
  • if already full, new synapses cannot be created
• synaptic decay plays (now) a role, and "zero" synapses can be pruned, making place for new ones.

The reason being:

• the new approach is biologically plausible, the old one not so much
• results remain +- the same
• syn decay now has a functionality (pruning synapses that are/became irrelevant). It's imo good to have used, not to have unused functionality in the code.

Where is the replacement code?

it had already existed, it's the synaptic decay, but only now it becomes more important.

The tests are very minimal.

as above, the code is already in place. I've reworked the existing test which was really a unit test tailored for the specific function (and not a "correct functionality test"). I was able to demonstrate we can do the same (even with finer control of how many times the event has to occur, for the syn to be replaced) with the already existing functionality.

So code-wise, this is a cleanup, and one functionality is performed by other part.

The code has been only relevant in "real data" hotgym.py (not HelloSPTP/sinewave hotgym in c++) where the results remain the same.

[ctrl-z-9000-times]

the new approach is biologically plausible, the old one not so much

Do you have any evidence for this?

results remain +- the same

I'm not convinced since the unit tests are very simple. For functional changes to the TM algorithm, the NAB is a better test.

not to have unused functionality in the code.

That's not a great reason to remove code that is used.

[dkeeney]

Hmmm, I wonder if I should have approved this if @ctrl-z-9000-times has an objection.

[breznak]

Hmmm, I wonder if I should have approved

you can change your Review, and I'm not going to merge (yet) until we discuss the aformentioned concerns, so no worries.

[breznak]

results remain +- the same

I'm not convinced since the unit tests are very simple. For functional changes to the TM algorithm, the NAB is a better test.

fair point, thanks! As for the tests, I'll validate on hotgym.py that actually uses (real, complex) data. And on NAB. A problem with NAB is that in the current state, our HTM.core is still underperforming the Numenta's HTM (bad params?), so all I can do is compare the performance in this PR with the current master, would that be OK?

the new approach [decay] is biologically plausible, the old one [removing min permanence synapses when needed] not so much

Do you have any evidence for this?

I'll try to point to some research, to be fair, we don't have here listed evidence for the current approach either.

Note: a big missing NO-NO in this PR is that I forgot to implement the actuall DECAY I've been talking about here. So..let me code that :)

[ctrl-z-9000-times]

all I can do is compare the performance in this PR with the current master, would that be OK?

Yes, that would be fine.

I'll try to point to some research, to be fair, we don't have here listed evidence for the current approach either.

I know, the current approach seems pragmatic rather than biological.

[breznak]

all I can do is compare the performance in this PR with the current master, would that be OK?

FYI, I ran the tests, so far only without the destroyMinPermanenceSynapses (alternative decay not yet implemented) and the results on NAB show that that functionality is very rarely used (no significant effect) :

NAB

master

Running score normalization step
Final score for 'htmcore' detector on 'standard' profile = 63.86
Final score for 'htmcore' detector on 'reward_low_FP_rate' profile = 60.12
Final score for 'htmcore' detector on 'reward_low_FN_rate' profile = 66.71
Final scores have been written to /mnt/store/devel/HTM/NAB/results/final_results.json.

PR (no decay yet):

Running score normalization step
Final score for 'htmcore' detector on 'standard' profile = 63.37
Final score for 'htmcore' detector on 'reward_low_FP_rate' profile = 59.15
Final score for 'htmcore' detector on 'reward_low_FN_rate' profile = 66.39

PR (with decay implemented)

TBD