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

fixes multi run with resets and sets #299

Closed
wants to merge 2 commits into from
Closed

Conversation

alan-stokes
Copy link
Contributor

fixes for discussion in
#298

@alan-stokes alan-stokes added the bug There is a fault with the code label Mar 19, 2018
@alan-stokes alan-stokes added this to the 5.0.0 milestone Mar 19, 2018
@coveralls
Copy link

coveralls commented Mar 19, 2018

Coverage Status

Coverage increased (+0.006%) to 42.582% when pulling 306ccd9 on fix_reset_for_pynn into a9a0ed5 on master.

@rowleya
Copy link
Member

rowleya commented Mar 19, 2018

This looks a little extreme now - it generates data every time, even when doing run, run, run. This will also then reset plastic weights, and even regenerate random weights just after a run, which is not correct. From #298 we are told that the network should stay the same, but the state variables should be reset. Thus the solution is probably to actually reset the state variables, which will then result in a reloading of data. The only thing left then is to set the time of the simulation back to 0, which I guess would happen if we reload the binaries again.

So in summary, the changes needed would be:

  1. Remove the loading of the original data on reset
  2. Remember the initial setting of the state variables just before run
  3. On reset, reset the state variables back to their initial setting

These changes should result in the code realising that a variable has changed and thus calling the DsgRegenerateRegions function.

@rowleya
Copy link
Member

rowleya commented Mar 19, 2018

OK, just reread #299 - plasticity should be reset also. This is very difficult with our system at present as we would regenerate connectivity, which isn't correct. Basically after a reset, you would be running a different netlist (i.e. same projections but different neurons would be connected). For this, we would have to always generate a seed, and keep that each time to regenerate the network. Alternatively, we would need to keep the weights at the start and then reload only if the connection is plastic.

@rowleya
Copy link
Member

rowleya commented Mar 19, 2018

So, here is a list of things that would make this work (extending the above list):

  1. Remove the loading of the original data on reset
  2. Remember the initial setting of the state variables just before run
  3. On reset, reset the state variables back to their initial setting; this will make the vertex reload the region with this change.
  4. Update the code to support changes to the connectivity (but not adding new projections, just changes to weights, delays, connectors and synapse type); make the synaptic matrix region reload when changed.
  5. Remember the first generation of connectivity, weights and delays. Once these are generated on the machine, they need to be recalled from the machine before run(!)
  6. On reset, reset the synaptic matrix back to the original values (note this now becomes a from list connector of the original connectivity).

Note that points 4-6 are hard! Doing 1-3 would fix the current issues, but wouldn't do reset properly.

@alan-stokes
Copy link
Contributor Author

it is worth noting that reset currently does a stop app, so you cant just reload. you need to generate all the data regions again,as they've been nuked

@alan-stokes
Copy link
Contributor Author

  1. Remove the loading of the original data on reset (i dont see that in reset anyhows). And pynn 0.8/0.9 doesnt do owt but a cache of recorded data before calling reset. therefore i think you're mental state of what reset does is off (which is fine, so was mine until i dug into the code to verify what we do currently).

@alan-stokes
Copy link
Contributor Author

ok, given your view (i hadnt tested this code, and didnt think about it much). The easiest option is to do the following.

on run after reset, go through the run folders and reload the dsg's that are there. that'll reload the sets, but the initial values will be what was done on the first call to run. Wich emans we cant delete the reloaded dsg regions

@rowleya
Copy link
Member

rowleya commented Mar 19, 2018

OK, so looking at this, what we really need to do is much simpler, and that is just points 2 and 3 above. To repeat, the list now becomes:

  1. Remember the initial setting of the state variables just before run
  2. On reset, reset the state variables back to their initial setting; this will make the vertex reload the region with this change.

That sorts out all the problems. Note everything else is left as it is now in master i.e. not in this branch.

@rowleya
Copy link
Member

rowleya commented Mar 19, 2018

Actually this works with the current code, though not necessarily for the right reasons. We should also set all the values of the parameters to their current values again. This will mark the region dirty and cause it to regenerate and reload (even though it will regenerate the current values again). This would mean that even if we split state variables from parameters later on, this code would continue to work.

@alan-stokes
Copy link
Contributor Author

unknown what we want to do with this now?

@rowleya
Copy link
Member

rowleya commented May 23, 2018

Hmm, I can't remember if this got fixed elsewhere, or whether we just talked about it...

@alan-stokes
Copy link
Contributor Author

shrugs dont know.

@rowleya
Copy link
Member

rowleya commented Jul 18, 2018

A quick test shows that this doesn't actually work currently; it ends up calling data specification twice I believe.

@rowleya
Copy link
Member

rowleya commented Jul 5, 2019

This is now fixed in #450, so closing this PR.

@rowleya rowleya closed this Jul 5, 2019
@rowleya rowleya deleted the fix_reset_for_pynn branch July 5, 2019 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug There is a fault with the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants