search

SpiNNakerManchester / sPyNNaker

The SpiNNaker implementation of the PyNN neural networking language
Apache License 2.0
105 stars 43 forks source link

Typo in STDP refactor #42

Closed neworderofjamie closed 9 years ago

neworderofjamie commented 9 years ago

Looks like there should be an underscore on the 2nd nearest:

File "/usr/local/lib/python2.7/dist-packages/spynnaker/pyNN/models/neural_properties/synapse_dynamics/dependences/spike_pair_time_dependency.py", line 34, in eq and (self._nearest == other.nearest))

Suggested test cases to catch this:

1) 2 incoming projections to a population each with STDP rules with different dependence types and/or parameters - Should fail in a controlled manner 2) 2 incoming projections to a population each with STDP rules with matching dependence types and parameters - Should work perfectly

Cheers

Jamie

neworderofjamie commented 9 years ago

In fact a lot of those comparison functions look to be missing underscores e.g. in AdditiveWeightDependence and PfisterSpikeTripletTimeDependence. Also STDPMechanism.equals isn't implemented so it probably shouldn't be called from STDPMechanism.eq..

alan-stokes commented 9 years ago

Hi Jamie, if this is in the python. I aguely renemebr i long time ago, giving these things getters (thus no _ needed) to sort out the issue that your trying to access private variables of other.

this change was ages ago, and so im persuming that it hasnt been broken for that long. please correct me if im wrong.

regards Alan

neworderofjamie commented 9 years ago

OK, but after that, similar errors here (a whole block of them) e.g.:-

File "/usr/local/lib/python2.7/dist-packages/spynnaker/pyNN/models/neural_properties/synapse_dynamics/stdp_mechanism.py", line 87, in eq == other.dendritic_delay_fraction) AttributeError: 'STDPMechanism' object has no attribute 'dendritic_delay_fraction'

If I fix these I then move onto :-

==Traceback (most recent call last): File "/home/ad246/cuda-workspace/Spinnaker MNIST Classifier/src/python/stdp_example.py", line 180, in sim.run(simtime) File "/usr/local/lib/python2.7/dist-packages/spynnaker/pyNN/init.py", line 180, in run _spinnaker.run(run_time) File "/usr/local/lib/python2.7/dist-packages/spynnaker/pyNN/spinnaker.py", line 166, in run self.map_model() File "/usr/local/lib/python2.7/dist-packages/spynnaker/pyNN/spinnaker.py", line 384, in map_model self._execute_partitioner(pacman_report_state) File "/usr/local/lib/python2.7/dist-packages/spynnaker/pyNN/spinnaker.py", line 467, in _execute_partitioner self._machine) File "/usr/local/lib/python2.7/dist-packages/pacman/operations/partition_algorithms/partition_and_place_partitioner.py", line 129, in partition vertex, subgraph, graph_mapper, machine, graph) File "/usr/local/lib/python2.7/dist-packages/pacman/operations/partition_algorithms/partition_and_place_partitioner.py", line 206, in _partition_vertex machine) File "/usr/local/lib/python2.7/dist-packages/pacman/operations/partition_algorithms/partition_and_place_partitioner.py", line 250, in _partition_by_atoms max_atoms_per_core, graph) File "/usr/local/lib/python2.7/dist-packages/pacman/operations/partition_algorithms/partition_and_place_partitioner.py", line 317, in _scale_down_resources graph) File "/usr/local/lib/python2.7/dist-packages/pacman/model/partitionable_graph/abstract_partitionable_vertex.py", line 133, in get_resources_used_by_atoms sdram_requirement = self.get_sdram_usage_for_atoms(vertex_slice, graph) File "/usr/local/lib/python2.7/dist-packages/spynnaker/pyNN/models/abstract_models/abstract_partitionable_population_vertex.py", line 50, in get_sdram_usage_for_atoms

alan-stokes commented 9 years ago

Hi Jamie,

So i started looking at this and got royally confused by the results from the tool chain.

I basically built a synfire chain, which was to feed the same base neuron in the chain from different spike source arrays. Each array had a projection with a different STDP mechanism (different timing dependence or a different weight dependance) as described in your description.

you can find the test in spynnaker augmented tests brnach under intergration_tests/tests_for_stdp/test/test_multiple_stdp_mechs_on_same_neuron.py

Now during partitioning the tool chain produces this error emssage:

spynnaker.pyNN.exceptions.SynapticConfigurationException: Different STDP mechanisms on the same vertex are not supported

Does this mean that this issue is actually a none starter, as we currently dont support multiple STDP mechanisms on a vertex, and so these errors will never occur?

regards Alan

neworderofjamie commented 9 years ago

As discussed in my SpiNNaker meeting talk (!) we currently have no support for multiple incoming STDP projections of different types. However, multiple STDP projections with compatible parameters should work - The bug in question was within the code for checking that the parameters were compatible

On 15 April 2015 at 16:38, Alan Stokes notifications@github.com wrote:

Hi Jamie,

So i started looking at this and got royally confused by the results from the tool chain.

I basically built a synfire chain, which was to feed the same base neuron in the chain from different spike source arrays. Each array had a projection with a different STDP mechanism (different timing dependence or a different weight dependance) as described in your description.

you can find the test in spynnaker augmented tests brnach under intergration_tests/tests_for_stdp/test/test_multiple_stdp_mechs_on_same_neuron.py

Now during partitioning the tool chain produces this error emssage:

spynnaker.pyNN.exceptions.SynapticConfigurationException: Different STDP mechanisms on the same vertex are not supported

Does this mean that this issue is actually a none starter, as we currently dont support multiple STDP mechanisms on a vertex, and so these errors will never occur?

regards Alan

— Reply to this email directly or view it on GitHub https://github.com/SpiNNakerManchester/sPyNNaker/issues/42#issuecomment-93453662 .

alan-stokes commented 9 years ago

Hi Jamie,

I completly understand that, but as this issue was raised for the capo milestone, and as the tool chain currently says it doesnt support multiple STDP mechanisms on the same vertex, and that code thus checks this correctly. Thus i can push this issue to the sept milestone? as itll need your seperated synaptic cores for it to work, and thus is a lot of work which will not be avilable by tuesday next week. And untill the tool chain allows multiple stdp mechs on the same vertex, the bugs in the code this issue covers wont ever be executed.

neworderofjamie commented 9 years ago

Alan, I think you're missing the point - If you have a population with multiple compatible STDP rules, they aren't getting merged right - The checking is NOT working correctly.

On 17 April 2015 at 10:52, Alan Stokes notifications@github.com wrote:

Hi Jamie,

I completly understand that, but as this issue was raised for the capo milestone, and as the tool chain currently says it doesnt support multiple STDP mechanisms on the same vertex, and that code thus checks this correctly. Thus i can push this issue to the sept milestone? as itll need your seperated synaptic cores for it to work, and thus is a lot of work which will not be avilable by tuesday next week. And untill the tool chain allows multiple stdp mechs on the same vertex, the bugs in the code this issue covers wont ever be executed.

— Reply to this email directly or view it on GitHub https://github.com/SpiNNakerManchester/sPyNNaker/issues/42#issuecomment-93955357 .

alan-stokes commented 9 years ago

Hi Jamie,

I probabily am missing the point. I think your expecting too much background information on STDP rules. I dont know which rules are compatible with each other. Thus I dont know when that error message is valid or is not valid. I pointed you to the test script i built to test your hypthosis so that you could verify if it was indeed a test case that would represent the error case.

I concluded that we dont support fast STDP when it threw me the error saying so, so i reduced the test case so that it covered only slow stdp mechanisms.

And given that the next error message is "Different STDP mechanisms on the same vertex are not supported" this reads as no 2 different STDP mechanisms are mergable.

Now im not going to learn your PhD so that I can deduce what stdp rules are compatible or not, I just dont have the time. Youll need to walk me through this, given I have barely any neural knowledge, and about as much STDP knwoeldge as a dounut.

Therefore can you have a look at the script in intergration_tests/tests_for_stdp/test/test_multiple_stdp_mechs_on_same_neuron.py on the augmented_test branch of spynnaker and tell me if its a valid thing or not. Because currently I have no idea, and thus cannot help you further untill you guild myself though these things. Your the expert here, not myself, i know where my limitations are and am asking for you to just be a bit more understanding to my own capabilities.

If its a quick fix, then your welcome to fix it yourself in a brnach and make a pull request.

neworderofjamie commented 9 years ago

I have replaced that integration test in the multiple_stdp branch - Hopefully it is now clear what goes wrong..

On 20 April 2015 at 09:22, Alan Stokes notifications@github.com wrote:

Hi Jamie,

I probabily am missing the point. I think your expecting too much background information on STDP rules. I dont know which rules are compatible with each other. Thus I dont know when that error message is valid or is not valid. I pointed you to the test script i built to test your hypthosis so that you could verify if it was indeed a test case that would represent the error case.

I concluded that we dont support fast STDP when it threw me the error saying so, so i reduced the test case so that it covered only slow stdp mechanisms.

And given that the next error message is "Different STDP mechanisms on the same vertex are not supported" this reads as no 2 different STDP mechanisms are mergable.

Now im not going to learn your PhD so that I can deduce what stdp rules are compatible or not, I just dont have the time. Youll need to walk me through this, given I have barely any neural knowledge, and about as much STDP knwoeldge as a dounut.

Therefore can you have a look at the script in intergration_tests/tests_for_stdp/test/test_multiple_stdp_mechs_on_same_neuron.py on the augmented_test branch of spynnaker and tell me if its a valid thing or not. Because currently I have no idea, and thus cannot help you further untill you guild myself though these things. Your the expert here, not myself, i know where my limitations are and am asking for you to just be a bit more understanding to my own capabilities.

If its a quick fix, then your welcome to fix it yourself in a brnach and make a pull request.

— Reply to this email directly or view it on GitHub https://github.com/SpiNNakerManchester/sPyNNaker/issues/42#issuecomment-94390772 .

rowleya commented 9 years ago

Split in to #66 and #67