IGITUGraz / spore-nest-module

Synaptic Plasticity with Online Reinforcement learning
https://igitugraz.github.io/spore-nest-module/
GNU General Public License v2.0
25 stars 11 forks source link

Test and merge python3 branch with master #8

Closed anandtrex closed 7 years ago

anandtrex commented 7 years ago

Tracking/reminder to do the merge for myself.

kappeld commented 7 years ago

I briefly looked at the diff between master and py3. I don't think it should be too much work. There are a lot of changes but they are (almost) mutually exclusive. Everything up to examples/pattern_matching_showcase/python/ should be already python3 ready in master, since I tried out the 2to3 tool there, which leaves only the deeper folders "experiment_utils" and "snn_utils". If you want we can take a look at this together some time that is convenient with you.

mhoff commented 7 years ago

You can leave making examples/pattern_matching_showcase/python/ python 2/3 to me. I will also have to do this with my current project code which is very similar, so it makes more sense for me to do it.

mhoff commented 7 years ago

@anandtrex could you test the latest commits?

178a18494a173fd9834a9a8c2f81b9487194bebb: See if cmake -Dwith-python=3 .. does properly affect make test. 4907eb529eaac26718bdcb5e288cbb45a868d0d9: Does the experiment work with python3?

mhoff commented 7 years ago

Everything seems to work now. I'm puzzled by the performance @anandtrex achieved on his machine.

On a pcluster node:

t = 240.2550: pattern =  1, reward = 0.0200 (~0.0208), speedup = 50.2175, rates = [  4.  12.]
t = 255.2700: pattern =  1, reward = 1.5715 (~0.0208), speedup = 50.2188, rates = [ 22.  14.]
t = 270.2850: pattern =  0, reward = 0.0200 (~0.0211), speedup = 50.1895, rates = [ 38.  26.]
t = 285.3000: pattern =  2, reward = 0.0200 (~0.0209), speedup = 50.1900, rates = [ 26.  14.]
t = 300.3150: pattern =  1, reward = 0.0200 (~0.0212), speedup = 50.1946, rates = [ 14.  16.]
t = 315.3300: pattern =  0, reward = 0.0200 (~0.0213), speedup = 50.1662, rates = [ 12.  10.]

With the same configuration, but on another machine and python3, @anandtrex got a speedup of 70-90 and at t = 300.. already an average reward of 0.07... (please correct me if I remember that wrong). I see no reason for the learning to be that much faster?!

mhoff commented 7 years ago

Also, we could integrate afcf63020b5add36a7bf9859dbec2a3697530f44 and 7b091f66710022a47521ad1d648729584e006d75 into master.

kappeld commented 7 years ago

Sure, merging this into master looks fine to me.

@anandtrex and I resolved the mystery of the surprisingly high performance. This was just again related to issue #696 in NEST. The spikes of one of the populations was just not delivered to the control node, and then it is easy to get a quite high reward if the other population just learns to produce a large output all the time. This is a much simpler learning problem and thus learned much faster. On a single thread the learning behaviour is similar to the python 2.7 result.

anandtrex commented 7 years ago

@mhoff Should I cherry-pick those two commits into master? Or do we want to do a full merge?

mhoff commented 7 years ago

No full merge. The first one can be easily cherry-picked. The second one is mostly formatting. Did you do this by hand or a script? In the case of a script you can easily run this again. If by hand cherry-picking would make sense, however I fear some merge conflicts might come up. We have to try it out then.

mhoff commented 7 years ago

@anandtrex and I resolved the mystery of the surprisingly high performance. This was just again related to issue #696 in NEST. The spikes of one of the populations was just not delivered to the control node, and then it is easy to get a quite high reward if the other population just learns to produce a large output all the time. This is a much simpler learning problem and thus learned much faster. On a single thread the learning behaviour is similar to the python 2.7 result.

@kappeld Weird, though. I thought we were using a single thread. I remember that @anandtrex and I spoke about "nest_n_threads" and that we set it to 1.

@anandtrex Will you do the cherry-picks?

kappeld commented 7 years ago

When merging https://github.com/IGITUGraz/spore-nest-module/commit/7b091f66710022a47521ad1d648729584e006d75 I think it would be best to place the run-style-check.sh script in the utils/ folder.

mhoff commented 7 years ago

Agreed.

mhoff commented 7 years ago

@anandtrex Branch py3 can be removed, right?

anandtrex commented 7 years ago

Yup