Connections: replace destroyMinPermSynapses() with plausible decay #751

breznak · 2019-11-12T16:13:12Z

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

and adapts TM which used this function. The functionality is removed as not plausible biologically, instead we let a (weak) synapse decay even further, so it's removed.

this is adapted to the artificial destroyMinPermSynapses() removed, instead we rely on decay synapse to eventually remove the unused syn.

…pses

breznak

Please review @dkeeney @ctrl-z-9000-times , part of my progress PRs on making Connections more biological and faster at the same time

breznak · 2019-11-12T16:14:47Z

src/htm/algorithms/Connections.hpp

-   * @param nDestroy - Must be greater than or equal to zero!
-   * @param excludeCells - Presynaptic cells which will NOT have any synapses destroyed.
-   */
-  void destroyMinPermanenceSynapses(const Segment segment, Int nDestroy,


removed as not entirely bio-inspired, using synaptic decay instead.

PS: I also hope for some better cross platform determinism, as the simpler code does not depend so much on ordering of synapses.

removed as not entirely bio-inspired, using synaptic decay instead.

I now know what you're upto, and what was actually missing from this PR! the decay itself.
I'm gonna implement it for the synapses' updates.

src/htm/algorithms/TemporalMemory.cpp

src/test/unit/algorithms/TemporalMemoryTest.cpp

breznak · 2019-11-23T21:48:58Z

pingity-pong, please :)

ctrl-z-9000-times

I don't really understand what this change does.
Where is the replacement code?
The tests are very minimal.

ctrl-z-9000-times

double post

ctrl-z-9000-times

double post

breznak · 2019-11-25T11:55:15Z

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 · 2019-11-26T04:51:08Z

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.

…pses

dkeeney · 2020-02-03T21:43:17Z

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

breznak · 2020-02-03T21:45:52Z

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 · 2020-02-04T12:26:52Z

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 · 2020-02-04T23:57:52Z

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.

…pses

breznak · 2020-02-05T15:15:27Z

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

…pses

from those "least". Not random. Slight optimization

dkeeney

ok

breznak added 3 commits November 12, 2019 11:43

Connections: remove destroyMinPermSynapses

ef50d2d

and adapts TM which used this function. The functionality is removed as not plausible biologically, instead we let a (weak) synapse decay even further, so it's removed.

TM test: adapt to verify weak synapses get removed eventually

592fc4f

this is adapted to the artificial destroyMinPermSynapses() removed, instead we rely on decay synapse to eventually remove the unused syn.

Merge branch 'master_community' into connection_rm_destroyMinPermSyna…

1c0cf35

…pses

breznak added TM code code enhancement, optimization, cleanup..programmer stuff labels Nov 12, 2019

breznak requested review from dkeeney and ctrl-z-9000-times November 12, 2019 16:13

breznak self-assigned this Nov 12, 2019

breznak commented Nov 12, 2019

View reviewed changes

breznak added the ready label Nov 12, 2019

breznak mentioned this pull request Nov 19, 2019

Connections remove destroying LRU segments #758

Open

ctrl-z-9000-times reviewed Nov 25, 2019

View reviewed changes

breznak added 3 commits February 3, 2020 18:49

Merge branch 'master_community' into connection_rm_destroyMinPermSyna…

3712507

…pses

TM.growSynapses_() improve comment

d8fd6b6

TM Test: fix merge mistake

a2b8a33

dkeeney previously approved these changes Feb 3, 2020

View reviewed changes

breznak removed the ready label Feb 4, 2020

breznak added the in_progress label Feb 5, 2020

Merge branch 'master_community' into connection_rm_destroyMinPermSyna…

c67f9d8

…pses

Merge branch 'master_community' into connection_rm_destroyMinPermSyna…

491598d

…pses

TM getLeastUsedCells_() return 1st least used cell

90971c1

from those "least". Not random. Slight optimization

breznak dismissed dkeeney’s stale review via 90971c1 June 4, 2020 18:51

dkeeney approved these changes Jun 4, 2020

View reviewed changes

Zbysekz closed this Jun 26, 2020

Zbysekz deleted the connection_rm_destroyMinPermSynapses branch June 26, 2020 06:57

breznak restored the connection_rm_destroyMinPermSynapses branch June 26, 2020 07:05

breznak reopened this Jun 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connections: replace destroyMinPermSynapses() with plausible decay #751

Connections: replace destroyMinPermSynapses() with plausible decay #751

breznak commented Nov 12, 2019 •

edited

Loading

breznak left a comment

breznak Nov 12, 2019

breznak Feb 4, 2020

breznak commented Nov 23, 2019

ctrl-z-9000-times left a comment

ctrl-z-9000-times left a comment •

edited

Loading

ctrl-z-9000-times left a comment •

edited

Loading

breznak commented Nov 25, 2019

ctrl-z-9000-times commented Nov 26, 2019

dkeeney commented Feb 3, 2020

breznak commented Feb 3, 2020

breznak commented Feb 4, 2020

ctrl-z-9000-times commented Feb 4, 2020

breznak commented Feb 5, 2020

dkeeney left a comment

Connections: replace destroyMinPermSynapses() with plausible decay #751

Are you sure you want to change the base?

Connections: replace destroyMinPermSynapses() with plausible decay #751

Conversation

breznak commented Nov 12, 2019 • edited Loading

breznak left a comment

Choose a reason for hiding this comment

breznak Nov 12, 2019

Choose a reason for hiding this comment

breznak Feb 4, 2020

Choose a reason for hiding this comment

breznak commented Nov 23, 2019

ctrl-z-9000-times left a comment

Choose a reason for hiding this comment

ctrl-z-9000-times left a comment • edited Loading

Choose a reason for hiding this comment

ctrl-z-9000-times left a comment • edited Loading

Choose a reason for hiding this comment

breznak commented Nov 25, 2019

ctrl-z-9000-times commented Nov 26, 2019

dkeeney commented Feb 3, 2020

breznak commented Feb 3, 2020

breznak commented Feb 4, 2020

ctrl-z-9000-times commented Feb 4, 2020

breznak commented Feb 5, 2020

NAB

master

PR (no decay yet):

PR (with decay implemented)

dkeeney left a comment

Choose a reason for hiding this comment

breznak commented Nov 12, 2019 •

edited

Loading

ctrl-z-9000-times left a comment •

edited

Loading

ctrl-z-9000-times left a comment •

edited

Loading