Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stochastic neurons #1424

Merged
merged 39 commits into from
Apr 23, 2024
Merged

Stochastic neurons #1424

merged 39 commits into from
Apr 23, 2024

Conversation

rowleya
Copy link
Member

@rowleya rowleya commented Dec 19, 2023

Adds a couple of stochastic neuron models that might make WTA and Travelling Salesman problems work better

@coveralls
Copy link

coveralls commented Dec 19, 2023

Coverage Status

coverage: 69.462% (-0.07%) from 69.531%
when pulling b7be66a on stochastic_neurons
into 0a9574e on master.

@rowleya rowleya marked this pull request as ready for review March 15, 2024 11:11
@rowleya
Copy link
Member Author

rowleya commented Mar 15, 2024

I think this is ready now - the models work as they are

Copy link
Member

@Christian-B Christian-B left a comment

Choose a reason for hiding this comment

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

Some things to address before looking at the actual changes

@@ -0,0 +1,24 @@
# Copyright (c) 2017 The University of Manchester
Copy link
Member

Choose a reason for hiding this comment

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

  1. New but dated 2017?
  2. Not built

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@@ -0,0 +1,24 @@
# Copyright (c) 2014 The University of Manchester
Copy link
Member

Choose a reason for hiding this comment

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

  1. new but dated 2014
  2. Not built

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

threshold_type = ThresholdTypeFixedProb(v_thresh, p_thresh, seed)
super().__init__(
model_name="IF_curr_delta_fixed_prob",
binary="IF_curr_delta_fixed_prob.aplx",
Copy link
Member

Choose a reason for hiding this comment

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

Never built!

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed and test added

@@ -0,0 +1,73 @@
# Copyright (c) 2017 The University of Manchester
Copy link
Member

Choose a reason for hiding this comment

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

1, new but dated 2017
2. Never tested

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed and test added

threshold_type = ThresholdTypeStatic(v_thresh)

super().__init__(
model_name="IF_trunc_delta", binary="IF_trunc_delta.aplx",
Copy link
Member

Choose a reason for hiding this comment

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

Does not exists!

Copy link
Member Author

Choose a reason for hiding this comment

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

Added build and test added

@@ -0,0 +1,27 @@
# Copyright (c) 2015 The University of Manchester
Copy link
Member

Choose a reason for hiding this comment

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

date

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@@ -0,0 +1,31 @@
# Copyright (c) 2015 The University of Manchester
Copy link
Member

Choose a reason for hiding this comment

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

date

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@@ -0,0 +1,88 @@
# Copyright (c) 2015 The University of Manchester
Copy link
Member

Choose a reason for hiding this comment

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

date

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@@ -0,0 +1,73 @@
# Copyright (c) 2017 The University of Manchester
Copy link
Member

Choose a reason for hiding this comment

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

Date

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

@Christian-B Christian-B left a comment

Choose a reason for hiding this comment

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

some more comments.
Especially double recording yet only sdram space for 1
possible double spike

return ukbits(accumulator);
}

static const uint64_t MAX_REAL = 0xFFFFFFFF0000L;
Copy link
Member

Choose a reason for hiding this comment

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

unused

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed


static const uint64_t MAX_REAL = 0xFFFFFFFF0000L;

static const uint64_t MIN_REAL = 0x000000010000L;
Copy link
Member

Choose a reason for hiding this comment

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

also unused

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed

SOMETIMES_UNUSED // Marked unused as only used sometimes
static bool neuron_impl_initialise(uint32_t n_neurons) {
// Allocate DTCM for neuron array
if (sizeof(neuron_impl_t) != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

why do we need this check?
neuron_impl_t is defined locally so will never have size 0

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed

//! Array of neuron states
static neuron_impl_t *neuron_array;

SOMETIMES_UNUSED // Marked unused as only used sometimes
Copy link
Member

Choose a reason for hiding this comment

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

Unusual having all these SOMETIMES_UNUSED in an h file only included in one make file

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed from this file in general


// Record things
neuron_recording_record_int32(PROB_INDEX, neuron_index, 0);
neuron_recording_record_accum(V_RECORDING_INDEX, neuron_index, ZERO);
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to record these with the same index and time before and after spike?
Or better to use a different index

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't record twice - there was a continue but now updated to use else, and separated code into separate functions

uint32_t random = mars_kiss64_seed(neuron->random_seed);

// If the random number is less than the probability value, spike
if (random < prob) {
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a neuron could spike twice in one timestep.

Copy link
Member Author

Choose a reason for hiding this comment

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

See above

}

SOMETIMES_UNUSED // Marked unused as only used sometimes
static void neuron_impl_do_timestep_update(
Copy link
Member

Choose a reason for hiding this comment

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

same comments about index used twice, sdram and possible double spike

Copy link
Member Author

Choose a reason for hiding this comment

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

See above

}

SOMETIMES_UNUSED // Marked unused as only used sometimes
static void neuron_impl_do_timestep_update(
Copy link
Member

Choose a reason for hiding this comment

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

again indexes reused only sdram for 1 recording and two chances to spike

Copy link
Member Author

Choose a reason for hiding this comment

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

See above

REAL v_membrane = (neuron->bias + neuron->inputs[0]) - neuron->inputs[1];

// Record things
neuron_recording_record_accum(V_RECORDING_INDEX, neuron_index, v_membrane);
Copy link
Member

Choose a reason for hiding this comment

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

some timesteps will record twice.
yet neuron recorder sampling rates are 1
this will cause you to run out of sdram on a long run!

Copy link
Member Author

Choose a reason for hiding this comment

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

See above

Copy link
Member

@Christian-B Christian-B left a comment

Choose a reason for hiding this comment

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

looks ok pending Jenkins being green

@rowleya
Copy link
Member Author

rowleya commented Apr 23, 2024

Jenkins is green so merging!

@rowleya rowleya merged commit df7ce07 into master Apr 23, 2024
10 checks passed
@rowleya rowleya deleted the stochastic_neurons branch April 23, 2024 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants