Issue 179 compilation speedup

[SinYita]

Header separation

This PR involves the header file separation of objects.h and rand.h. It's a preparation work for later PIMPL approach.
For objects.h, we now have objects_thrust.h, which is in charge of Thrust containers declaration. They are explicitly imported in templates right now. These will later be replaced by objects_api.h (which provides an API for Thrust functionalities). The main idea is to let code objects only include the Thrust library when they indeed need it, instead of importing it everywhere.

Another change concerns the RNG in objects.h, which has now been moved into rand.h, making it responsible for RNG declarations / buffer / device states. Since parsing <curand.h> also takes a lot of time for a bunch of code objects, we only import it when necessary, just like what we did for Thrust. The main approach is similar to what we have achieved in cuda_generator.py.
A better long-term approach would be making Brian2's BinomialFunction to forward compiler_kwds.

The modification was tested with examples/mushroombody.py with monitors on and N=1000 by manually testing the build time of the artifacts.

Other minor modifications include adding the explicit namespace for std:: functions and adding random seed configurations for stdp.py and mushroombody.py.

Outcomes:

The -j1 compilation time reduced from 273.2s to 223.0s. For -j12 compilation time, it dropped from 31.65s to 28.08s.

Limits

Since we have thrust functionalities in a lot code objects. Splitting the header file will only provides a small acceleration.
A follow up PR would be PIMPL to hide the thrust and curand library for code objects, so that they won't parsing them and wasting time.

[mstimberg]

Hi @SinYita. Thanks for the PR! I've gone through the code and the general approach and its implementation look good to me. When I run the test suite, I get several errors, though. It seems as if at least the summed variable mechanism is broken, and also code that refers to synaptic variables in the equations or in run_regularly statements. Here are two specific failing tests that probably cover most of the issues (the test script is in brian2cuda/tools/test_suite):

$ python run_single_test.py --brian2 test_summed_variable
$ python run_single_test.py --brian2 test_state_monitor_synapses

If I am not mistaken, these are places where dynamic arrays are used in a simple read-only way, i.e. we just need the underlying data pointer. We can probably work around this by having a global variable with the pointer (as I think you are doing in the "PIMPL approach"), and make sure that it is updated before we call the relevant code object.

Fixing the test failures is of course the first priority, but maybe also double check whether there are places where objects_thrust.h is included unnecessarily. I think at least in reset.cu it is not needed – the mushroombody example runs fine if I remove it.

Finally, a positive thing from my side: on my machine, the speed advantage is bigger than on yours 😊 With -j16, my compilation time goes from 46.8s to 35.7s.

[SinYita]

Hi @SinYita. Thanks for the PR! I've gone through the code and the general approach and its implementation look good to me. When I run the test suite, I get several errors, though. It seems as if at least the summed variable mechanism is broken, and also code that refers to synaptic variables in the equations or in run_regularly statements. Here are two specific failing tests that probably cover most of the issues (the test script is in brian2cuda/tools/test_suite):

$ python run_single_test.py --brian2 test_summed_variable
$ python run_single_test.py --brian2 test_state_monitor_synapses

If I am not mistaken, these are places where dynamic arrays are used in a simple read-only way, i.e. we just need the underlying data pointer. We can probably work around this by having a global variable with the pointer (as I think you are doing in the "PIMPL approach"), and make sure that it is updated before we call the relevant code object.

Fixing the test failures is of course the first priority, but maybe also double check whether there are places where objects_thrust.h is included unnecessarily. I think at least in reset.cu it is not needed – the mushroombody example runs fine if I remove it.

Finally, a positive thing from my side: on my machine, the speed advantage is bigger than on yours 😊 With -j16, my compilation time goes from 46.8s to 35.7s.

Thank you Marcel, I will be working on fixing it.

[SinYita]

@mstimberg Hi Marcel, I just added objects_thrust.h for summed_variable, stateupdateand spatialstateupdate.cu. Also tested them with

$ python run_single_test.py --brian2 test_summed_variable
$ python run_single_test.py --brian2 test_state_monitor_synapses

100% passed.
I didn't remove objects_thrust.h" from reset.cu, as all the dynamic arrays are now declared inside objects_thrust.h. We need it for eventspace. PIMPL will solve the issue completely on the other PR. Sadly, this modification will increase the compilation time, but we will get it back.

[mstimberg]

@SinYita Hi Weiyuan.

I just added objects_thrust.h for summed_variable, stateupdateand spatialstateupdate.cu

Adding it unconditionally to stateupdate.cu and spatialstateupdate.cu might be a bit extreme, no? Now the state updater for a NeuronGroup will include it as well, even though it does not use any dynamic arrays. In my benchmark, the new version cuts the speed advantage in half, compilation now takes 41s.

I didn't remove objects_thrust.h" from reset.cu, as all the dynamic arrays are now declared inside objects_thrust.h. We need it for eventspace.

Eventspace is not a dynamic array, so this should not be a problem (also : I ran it without the include and it worked 😏 )

[SinYita]

@mstimberg Hi Marcel, Yes, it's quite aggressive to unconditionally add headers every file. My initial idea is to make this PR a header file splitting only. As you noticed, the scope drifts a bit. To keep things manageable, I just brought over a commit from the PIMPL branch so we can iterate on everything step-by-step in one place. I'll close the PIMPL branch/PR and consolidate all the changes into this PR eventually.

[SinYita]

Copied from the closed PR:
This PR introduces PIMPL as we talked in meetings, reducing the cost of parsing heavy header files like Thrust during compilation of various codeobjects. Specifically: objects_storage.h holds the actual Thrust container. The lean objects.h exposes raw pointers (such as dev_array_* / host_array_*). And the newly generated objects_api.h inside device.py provides resize/clear/copy, eventspace extensions, and wrappers for some algorithms and cuRAND-related code previously scattered across other TUs. Most .cu files access data via raw pointers and the API, concentrating Thrust usage primarily in objects.cu.

Pointers

sync_all_dev_ptrs() is called once at the end of _init_arrays() for a full refresh. If the container is modified via the API (such as resize_*), only the relevant pointers are synchronized. Therefore, as long as mutates use the API, the global PIMPL pointer can remain consistent with the container. The trade-off is the generation of a large number of API symbols in arrays(which is quite ugly right now). A more general template could be considered later to reduce the amount of generated code. And also I haven't apply PIMPL also for host vector, which will be add soon in following commits.

Outcome

On MushroomBody (N=1000), cold compilation with make -j1 is reduced to approximately 100s, with 'make -j12' is reduced to around 22s.(compared to approximately ~200s/~33s on master under the same environment RTX3080Ti 12GB).

Limitation

Since we moved all the thrust into objects.cu, the compilation of this TU increased from ~12s to ~19s and becomes the new bottleneck of parallel make task. Also the complexity of objects.cu increased, which gives heavy burden to host side. Applying nvcc --fdevice-time-trace to objects.cu shows gcc(compiling) occupies nearly 1/3 of the whole time.

[SinYita]

Latest force push revert changes about customer algorithm and tuples in synapses_push_spikes.cu. As the benefits are not distinguish but introducing complexity and being harder to maintain in future.