-
Notifications
You must be signed in to change notification settings - Fork 21
Description
I've been recently trying to understand and search through the CARLsim codebase for the parameters used in the COBA mode calculations. I have noticed several instances where coding style varies drastically, and sometimes to the detriment of my ability to search the codebase using regex (or even just a simple ctrl+f). As an example
CARLsim6/carlsim/kernel/src/snn_cpu_module.cpp
Line 1819 in d527c55
| runtimeData[netId].gAMPA [postNId] += change * mulSynFast[mulIndex]; // scale by some factor |
has a space between gAMPA and [postNId] which makes some searching challenging even though normally there shouldn't be any space between an array and the indexing.
Earlier in that file we can see other issues with curly brace formatting
CARLsim6/carlsim/kernel/src/snn_cpu_module.cpp
Lines 1330 to 1336 in d527c55
| if (dPar_old.delay_length >= 0) { | |
| dPar_old.delay_length--; // decrement | |
| dPar_old.delay_index_start = 0; // reset entry | |
| } | |
| else { | |
| KERNEL_ERROR("Post-synaptic delay was not sorted correctly pre_id=%d, offset=%d", lNIdPre, offset); | |
| } |
But later in that same method we see
CARLsim6/carlsim/kernel/src/snn_cpu_module.cpp
Lines 1362 to 1369 in d527c55
| if (offset == n) { | |
| setDelay(connInfo.nSrc, connInfo.nDest, connInfo.delay); // update delay cache | |
| } | |
| else | |
| { | |
| printf("WARNING: skipping setDelay (offset!=n): pre=%d, post=%d delay=%d\n", | |
| connInfo.nSrc, connInfo.nDest, connInfo.delay); | |
| } |
And later in that same file
CARLsim6/carlsim/kernel/src/snn_cpu_module.cpp
Lines 2740 to 2746 in d527c55
| if (lGrpId == ALL) { | |
| lengthN = networkConfigs[netId].numNAssigned; | |
| posN = 0; | |
| } else { | |
| lengthN = groupConfigs[netId][lGrpId].numN; | |
| posN = groupConfigs[netId][lGrpId].lStartN; | |
| } |
These are clearly small problems with the code base, but they create challenges for code readability and sometimes making understanding the code difficult (and challenges the ease of searching and traversing the codebase).
I'm also sure that there is talk about how many pieces of commented out code are covered throughout the entire repository. While these chunks of code appear to have been added long ago, there are some that are very newly added. I know they are additional debug statements, but CARLsim has a specific KERNEL_DEBUG for this (see d527c55) which I think is wonderful and helps keep the code clean.
I would really love to see some form of linter being implemented in the repository to clean it automatically. CARLsim is such a useful piece of software, and quite sprawling with features, that manually linting would be impractical, and there are plenty of tools that exist to set this up. I don't have much experience with linting C++ but I've linked one below that seems good (I've set it up for python projects, and I'm assuming the process is similar with just some slight nuances for the specific linter being used). It appears to have samples setup for using GitHub Actions as well, so hopefully that can simplify the transition as well!