Plateau Neuron - Fixed Point Model #781

kds300 · 2023-09-01T00:41:41Z

Issue Number: #775

Objective of pull request: Add Plateau neuron model to lava as described in the above issue.

Pull request checklist

Your PR fulfills the following requirements:

Issue created that explains the change and why it's needed
Tests are part of the PR (for bug fixes / features)
Docs reviewed and added / updated if needed (for bug fixes / features)
PR conforms to Coding Conventions
PR applys BSD 3-clause or LGPL2.1+ Licenses to all code files
Lint (flakeheaven lint src/lava tests/) and (bandit -r src/lava/.) pass locally
Build tests (pytest) passes locally

Pull request type

Please check your PR type:

What is the current behavior?

There is no current behavior, this is a new model.

What is the new behavior?

The model will take two separate inputs, one to the "soma" and another to the "dendrite". Both the soma and dendrite potentials will follow sub-threshold LIF dynamics. When the dendrite potential crosses its threshold it resets and sends the neuron to the UP state. The UP state will last for a specified duration of time steps. During this time, the soma is able to spike and reset if its potential crosses its threshold. If the neuron is in the DOWN state, the soma potential will follow subthreshold LIF dynamics regardless of whether it crosses the threshold.

Does this introduce a breaking change?

Yes
No

Supplemental information

Update to lava main 20230816

Signed-off-by: kevin <[email protected]>

Signed-off-by: Marcus G K Williams <Marcus G K Williams [email protected]>

mgkwill · 2023-10-18T06:51:13Z

Hi @kds300, Thanks for this PR. Sorry it took us so long to take a look.

Pushed a PR to kds300#7 that should fix the codacy issues.

I will do more review in the next few days and I've added a few others for comment and review.

mgkwill · 2023-10-18T06:24:32Z

src/lava/proc/plateau/models.py

+    def run_spk(self):
+        """The run function that performs the actual computation during
+        execution orchestrated bgy a PyLoihiProcessModel using the
+        LoihiProtocol.
+        """


typo in orchestrated bgy a PyLoihiProcessModel, should be orchestrated by a PyLoihiProcessModel

src/lava/proc/plateau/models.py

phstratmann · 2023-10-18T10:25:29Z

src/lava/proc/plateau/process.py

+class Plateau(AbstractProcess):
+    """Plateau Neuron Process.
+
+    Couples two modified LIF dynamics. The neuron posesses two potentials,


Thanks for providing such an extensive, well written docstring!

Is 'potential' the right word, or 'voltage'?

I'm not sure, I'm used to treating them (mostly) interchangeably. I can change it to voltage since that's more consistent with other neuron models.

phstratmann · 2023-10-18T10:27:01Z

src/lava/proc/plateau/process.py

+from lava.magma.core.process.ports.ports import InPort, OutPort
+
+
+class Plateau(AbstractProcess):


Just a minor point. Since this is a LIF neuron, did you consider to let it inherit from the AbstractLIF process, and adding a 'LIF' in the class name? Not sure if it makes sense in this specific example, though. Up to you.

I originally didn't do this since I thought of it as two combined LIF neurons instead of a modified LIF neuron. I'll look over the AbstractLIF process and see if it makes sense to inherit from that.

phstratmann · 2023-10-18T10:35:33Z

tests/lava/proc/plateau/test_models.py

+from lava.tests.lava.proc.lif.test_models import VecSendProcess, VecRecvProcess
+
+
+class SpikeGen(AbstractProcess):


Could you check whether the Spiker Process in
'lava\src\lava\proc\spiker\process.py'
already implements the behavior that you expect from SpikeGen?

Since these two processes are very similar, it would be good to avoid redundant processes.

Maybe better to extend the Spiker, inherit from it, or write a common abstract spiker, rather than having two independent processes.

I've replaced the custom SpikeGen process with implementing source RingBuffers. I didn't use the Spiker because that sends periodic spikes and I only wanted single spikes to be sent.

phstratmann · 2023-10-18T10:38:25Z

tests/lava/proc/plateau/test_models.py

+    def run_spk(self):
+        """Send the appropriate spikes for the given time step
+        """
+        self.s_out.send(self.spike_data[:, self.time_step - 1])


One of the advantages of event-based, neuromorphic computing is that we don't need to communicate '0's. Thus, could you check whether the SpikeGen actually sends a meaningful information at the respective time step, and ensure not to send anything instead of a 0?

I've removed this process, but I'll definitely keep an eye out for this in any future processes I make!

phstratmann · 2023-10-18T10:41:09Z

tests/lava/proc/plateau/test_models.py

@@ -0,0 +1,178 @@
+# Copyright (C) 2023 Intel Corporation


Thanks for providing extensive unit tests!

phstratmann · 2023-10-18T10:42:30Z

tests/lava/proc/plateau/test_models.py

+            0, 6400, 4800, 3600, 2700, 2025, 1518, 1138, 853, 639
+        ]
+        self.assertListEqual(expected_v_dend, test_v_dend)
+        self.assertListEqual(expected_v_soma, test_v_soma)


Could you check if lintin passes?
'flakeheaven lint src/lava tests'

My guess is that there are a few points that must change, including missing lines at the end of files. Not functionally relevant, but important to keep a clean code base :-)

I've run the linting, (flakeheaven and bandit) and they pass on my local copy of the code. Also, I have the empty line at the end of the files locally, but it doesn't seem to show up on the github versions. Does github just not show the empty line at the end?

phstratmann · 2023-10-18T10:45:04Z

src/lava/proc/plateau/models.py

+
+    Precisions of state variables
+
+    - du_dend : unsigned 12-bit integer (0 to 4095)


In the process, you specify that the variables are float. If you want them to be integers, I would specify that in the Process.
Also, is this model really using fixed point precision? Or is it bit accurate to Loihi?
In the later case, we tend to call it "...ModelBitAcc", rather than "...ModelFixed"

I wasn't sure if bit-accurate would be appropriate for this, since I don't have a version of this that runs on Loihi. I tried to give the variables the same precision as on Loihi.

phstratmann · 2023-10-18T10:45:29Z

src/lava/proc/plateau/models.py

+    Precisions of state variables
+
+    - du_dend : unsigned 12-bit integer (0 to 4095)
+    - du_soma : unsigned 12-bit integer (0 to 4095)


Please note: On Loihi, variables typically have 8, 16, or 24 bit precision. Thus, you may want to avoid 12bit.

I chose 12-bit for the du and dv values since that was the precision of those values in the PyLifModelBitAcct. Since I'm choosing the precisions based on bit-accurate models, should I change this model to bit-accurate, as suggested in a different comment?

phstratmann · 2023-10-18T10:47:15Z

src/lava/proc/plateau/models.py

+    - du_dend : unsigned 12-bit integer (0 to 4095)
+    - du_soma : unsigned 12-bit integer (0 to 4095)
+    - vth_dend : unsigned 17-bit integer (0 to 131071)
+    - vth_soma : unsigned 17-bit integer (0 to 131071)


Also, could you add input validation? I.e. a method that checks that the provided variables are in fact integer, and in the required range?

I know that we are also sloppy on this sometimes ;-)

I've added input validation for the dv_dend, dv_soma, vth_dend, vth_soma, and up_dur variables. With the restriction on the dv's being set to 4095, I also had to make some changes to the decay process, setting the decay_unity = 4095. I'm not sure if this model could still be considered bit-accurate at this point.

Also, I fixed the typo in the docstring referenced above, du_dend and du_soma should both be dv's

phstratmann

Thanks for your contribution. Great code quality!

Just a few minor points I would ask you to change.

Please reach out if you have any questions.

kds300 · 2023-10-18T14:15:17Z

Thanks for your contribution. Great code quality!

Just a few minor points I would ask you to change.

Please reach out if you have any questions.

Thanks for the feedback! I'll go through code and make the requested changes.

Signed-off-by: kevin <[email protected]>

…nd, and vth_soma Signed-off-by: kevin <[email protected]>

…nd, dv_soma, vth_dend, and vth_soma Signed-off-by: kevin <[email protected]>

Signed-off-by: kevin <[email protected]>

…ateauModelFixed docstring Signed-off-by: kevin <[email protected]>

…nd and dv_soma (0, 4095) Signed-off-by: kevin <[email protected]>

Signed-off-by: kevin <[email protected]>

kds300 · 2023-10-19T18:33:12Z

I updated my local branch with several of the recommended changes, and commented on some of the requested changes that I have questions about.

Fix codacy issues

kds300 · 2023-10-19T19:45:54Z

Hi @kds300, Thanks for this PR. Sorry it took us so long to take a look.

Pushed a PR to kds300#7 that should fix the codacy issues.

I will do more review in the next few days and I've added a few others for comment and review.

@mgkwill Thanks for fixing the codacy issues! I'm still a bit new to github, especially using it collaboratively, so I had started making changes before merging that pull request. I added a commit to sort out some of the merge conflicts I created and then merged the pull request.

Signed-off-by: kevin <[email protected]>

…plateau-neuron

kds300 and others added 3 commits August 16, 2023 12:23

Merge pull request #6 from lava-nc/main

0263677

Update to lava main 20230816

plateau- fixed point implementation of Plateau neuron model

d70b6e7

Signed-off-by: kevin <[email protected]>

Merge branch 'main' into plateau-neuron

3f649d1

kds300 changed the title ~~Plateau neuron~~ Plateau Neuron - Fixed Point Model Sep 5, 2023

kds300 mentioned this pull request Sep 5, 2023

Plateau Neuron Model for Simulating Dendritic Plateau Potentials #775

Open

Merge branch 'main' into plateau-neuron

6647107

mgkwill requested review from mgkwill, bamsumit, drager-intel and phstratmann October 18, 2023 06:29

Fix codacy issues

5db90d7

Signed-off-by: Marcus G K Williams <Marcus G K Williams [email protected]>

mgkwill mentioned this pull request Oct 18, 2023

Fix codacy issues kds300/lava#7

Merged

mgkwill reviewed Oct 18, 2023

View reviewed changes

mgkwill requested a review from tim-shea October 18, 2023 06:55

phstratmann reviewed Oct 18, 2023

View reviewed changes

src/lava/proc/plateau/models.py Show resolved Hide resolved

phstratmann reviewed Oct 18, 2023

View reviewed changes

phstratmann requested changes Oct 18, 2023

View reviewed changes

kds300 added 3 commits October 19, 2023 12:19

fixed typo 'bgy' -> 'by' in PyPlateauModelFixed.run_spk docstring

bab1fd1

Signed-off-by: kevin <[email protected]>

changed 'potential' to 'voltage' in the Plateau class docstring

5d8b10c

Signed-off-by: kevin <[email protected]>

changed variable types from float to int for dv_dend, dv_soma, vth_de…

1961780

…nd, and vth_soma Signed-off-by: kevin <[email protected]>

kds300 added 6 commits October 19, 2023 12:44

updated Plateau process unit test to specify integer inputs for dv_de…

4be4918

…nd, dv_soma, vth_dend, and vth_soma Signed-off-by: kevin <[email protected]>

added input validation for dv_dend, dv_soma, vth_dend, vth_soma, up_dur

70433d4

Signed-off-by: kevin <[email protected]>

fixed typos 'du_dend' -> 'dv_dend' and 'du_soma' -> 'dv_soma' in PyPl…

4b26480

…ateauModelFixed docstring Signed-off-by: kevin <[email protected]>

reduced decay_unity by 1 (4096 -> 4095) to agree with limits on dv_de…

fb217ba

…nd and dv_soma (0, 4095) Signed-off-by: kevin <[email protected]>

updated tests to agree with changes to dv_dend, dv_soma allowed range

d1c66f0

Signed-off-by: kevin <[email protected]>

Removed custom SpikeGen process and replaced with source RingBuffers

c469bd8

Signed-off-by: kevin <[email protected]>

kds300 added 2 commits October 19, 2023 15:43

Merge branch 'plateau-neuron' into plateau-neuron

e8a8215

Merge pull request #7 from mgkwill/plateau-neuron

6ccbf74

Fix codacy issues

kds300 added 2 commits October 22, 2023 15:58

fixed codacy issues

cea6296

Signed-off-by: kevin <[email protected]>

Merge branch 'plateau-neuron' of https://github.com/kds300/lava into …

6e3744d

…plateau-neuron

kds300 requested a review from phstratmann November 9, 2023 20:44

Merge branch 'main' into plateau-neuron

0c6bf24

mgkwill force-pushed the main branch from 527dcfa to 5ed88f7 Compare November 15, 2023 04:57

Merge branch 'main' into plateau-neuron

d2aed48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plateau Neuron - Fixed Point Model #781

Plateau Neuron - Fixed Point Model #781

kds300 commented Sep 1, 2023

mgkwill commented Oct 18, 2023 •

edited

Loading

mgkwill Oct 18, 2023

phstratmann Oct 18, 2023

kds300 Oct 18, 2023

phstratmann Oct 18, 2023

kds300 Oct 18, 2023

phstratmann Oct 18, 2023

phstratmann Oct 18, 2023

phstratmann Oct 18, 2023

kds300 Oct 19, 2023

phstratmann Oct 18, 2023

kds300 Oct 19, 2023

phstratmann Oct 18, 2023

phstratmann Oct 18, 2023

kds300 Oct 19, 2023

phstratmann Oct 18, 2023

kds300 Oct 19, 2023

phstratmann Oct 18, 2023

kds300 Oct 19, 2023

phstratmann Oct 18, 2023

phstratmann Oct 18, 2023

kds300 Oct 19, 2023

kds300 Oct 19, 2023

phstratmann left a comment

kds300 commented Oct 18, 2023

kds300 commented Oct 19, 2023

kds300 commented Oct 19, 2023 •

edited

Loading

		from lava.magma.core.process.ports.ports import InPort, OutPort


		class Plateau(AbstractProcess):

		from lava.tests.lava.proc.lif.test_models import VecSendProcess, VecRecvProcess


		class SpikeGen(AbstractProcess):


		Precisions of state variables

		- du_dend : unsigned 12-bit integer (0 to 4095)

Plateau Neuron - Fixed Point Model #781

Are you sure you want to change the base?

Plateau Neuron - Fixed Point Model #781

Conversation

kds300 commented Sep 1, 2023

Pull request checklist

Pull request type

What is the current behavior?

What is the new behavior?

Does this introduce a breaking change?

Supplemental information

mgkwill commented Oct 18, 2023 • 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

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

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

Choose a reason for hiding this comment

phstratmann left a comment

Choose a reason for hiding this comment

kds300 commented Oct 18, 2023

kds300 commented Oct 19, 2023

kds300 commented Oct 19, 2023 • edited Loading

mgkwill commented Oct 18, 2023 •

edited

Loading

kds300 commented Oct 19, 2023 •

edited

Loading