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

[CSTP] CSTP decay bug #18

Open
nmsutton opened this issue Dec 6, 2022 · 3 comments
Open

[CSTP] CSTP decay bug #18

nmsutton opened this issue Dec 6, 2022 · 3 comments

Comments

@nmsutton
Copy link
Contributor

nmsutton commented Dec 6, 2022

In connection-specific STP (CSTP) synaptic conductance decay processing (and it seems not in non-CSTP code) synapse decay was incorrectly applied by summing all conductance to a receptor and then computing decay (STP’s tau_d) once for each synapse. This often results in far too much decay because each synapse’s signal does not decay at a rate (tau_d) specific to the neuron types (pre- and post-synaptic neuron groups). One incorrect effect this causes is each synapse affects the decay of every other synapse signal given a post-synaptic neuron. Each connection’s (pre- and post-synaptic neuron type) synapses need to decay at a rate set for the pre and post neuron types and not be affected by any other connection.

NS has created a proposed bug fix for this issue in his forked HCO branch: here and the HCO main branch is here. He has worked on developer documentation on CSTP (link) and the bug fix code (link).

Bug fix code known issues:
(1) The code in NS' fork does not yet process Grid3D neuron sizes correctly. When creating a neuron group with my forked code, please use an integer to specify a neuron group size and not a Grid3D object. CARLsim's hello_world project uses Grid3D objects which need to be removed for the project to run successfully.

(2) The bug fix code has only been implemented in the GPU version of running CARLsim. Please do not yet use the CPU version for testing the bug fix.

(3) The bug fix has only been developed for CARLsim6 so far.

More details are in the post below.

@nmsutton
Copy link
Contributor Author

nmsutton commented Dec 6, 2022

More details include that in the code with the bug the decay is incorrectly applied in snn_gpu_module.cu lines such as:
runtimeDataGPU.gAMPA[nid] *= runtimeDataGPU.stp_dAMPA[lSId]
In the bug fix code this is replaced with lines such as
runtimeDataGPU.gAMPA[nid] += *ampa_ptr
where ampa_ptr is a pointer to the conductance for each connection (pre+post neuron group) that has nid as a post. That conductance is decayed at a rate specific to that connection's tau_d before it is added to the receptor's total conductance.

One of the problems the bug creates is that a connection in a simulation with no current to it and no synaptic weight causes all other connections to have their synaptic conductance decay faster. A unit test (ca3cstp_taud) has been made to test for this condition and is here. The bux fix code corrects this problem.

Another unit test has been made to test the processing time of the bug fix code. This is named ca3cstp_perf, and it checks the ca3_example_net project runs in less than 20 sec. The bug code ran in 6.4 sec on my PC and the bug fix code ran in 8.9 sec. Full scale CA3 network tests have shown 0%-21% more time needed with the bug fix code.

Further info about how the candidate bug fix code was designed is here.

Watch out for issue 24 when testing this. Always test with RK4 stepsize 40 for now.

Additional testing of the bug fix code to ensure it works as expected may be useful.

@nmsutton
Copy link
Contributor Author

nmsutton commented Jan 20, 2023

Update 01/19/23: To validate that the new CSTP code runs correctly, @jkopsick and @nmsutton have run tests comparing the CSTP code results to the CARLsim master branch without CSTP. This is with the same STP parameters (params) between branches, there being only one set of excitatory params and one set of inhibitory STP params, and every connection having STP. This eliminates differences in STP conditions between CSTP and nonCSTP.

The results have been that CSTP matches nonCSTP output except when a certain scale of network size and connectivity and/or time occurs in the simulations. CSTP has more calculations than nonCSTP in the process of computing receptor conductance decays. The some calc diff. are: in CSTP, each connection’s conductance is decayed then added to a total receptor conductance. In nonCSTP, the total receptor conductance is decayed without any connection-specific computations. The additional computations are affected by numerical precision. The doubles data type is used in CSTP code for the decay calculations to enhance precision. Given enough complexity, there are some differences due to that numerical precision.

In our assessments, Jeffrey and I have yet to see evidence that anything else is contributing to differences in results in STP calculations between CSTP and nonCSTP branches (e.g., no errors detected in the code update). Therefore in our assessment, it appears the CSTP bug-fix code is working correctly but refer to others to confirm this as well.

@nmsutton
Copy link
Contributor Author

As an update, @larsnm has provided the help to review my pull request to fix this bug and merged the pull request. Thanks very much @larsnm ! Therefore, on the CSTP (ca3net) branch this appears fixed but still has the 3 known issues listed in my original post here. I think it perhaps makes sense to leave this issue open until those known issues are also fixed so that users are aware of them. I am open to suggestions about leaving this open or closed.

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

No branches or pull requests

1 participant