arbor
arbor copied to clipboard
Tests for particle diffusion
Python tests are added to ensure the working of the diffusion of arbitrary particles in setups with 1-3 segments (cf. issues https://github.com/arbor-sim/arbor/issues/2084 and https://github.com/arbor-sim/arbor/issues/2145).
Besides showing that the diffusion seems to work if segments have the same radius, the tests that are added here also point to the fact that the diffusion across segments of different radius is still erroneous. Therefore, I left the tests for different radii commented until https://github.com/arbor-sim/arbor/issues/2145 has been resolved.
Hey @jlubo,
thanks for adding this. Could you run black .
and push the results? That'll make the Linter
check pass.
And flake8 .
:D
Okay, black
now passes (I had previously used an older version, seems that they changed something). Yeah, now flake8
is complaining... I'll try to fix that, too :)
Hey, made some comments for improvements, rest looks great, thanks.
Thanks @thorstenhater! I've revised the code according to the comments.
cscs-ci run daint-gpu
Uh-oh
subprocess.CalledProcessError: Command 'make' returned non-zero exit status 2.
Error:
/tmp/tmpqgxm6fo0/build/generated/diffusion/neuron_with_diffusion_gpu.cu(59): warning #177-D: function "arb::diffusion_catalogue::<unnamed>::multiply" was declared but never referenced
/tmp/tmpqgxm6fo0/build/generated/diffusion/synapse_with_diffusion_gpu.cu(74): error: identifier "n_" is undefined
/tmp/tmpqgxm6fo0/build/generated/diffusion/synapse_with_diffusion_gpu.cu(55): warning #177-D: function "arb::diffusion_catalogue::<unnamed>::multiply" was declared but never referenced
1 error detected in the compilation of "/tmp/tmpqgxm6fo0/build/generated/diffusion/synapse_with_diffusion_gpu.cu".
make[2]: *** [CMakeFiles/diffusion-catalogue.dir/build.make:183: CMakeFiles/diffusion-catalogue.dir/generated/diffusion/synapse_with_diffusion_gpu.cu.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:83: CMakeFiles/diffusion-catalogue.dir/all] Error 2
@boeschf could you take a look? I suspect the recent changes to event delivery clash with the diffusion handling:
if (ii_ < (end_ - begin_)) {
const auto tid_ = (begin_ + ii_)->mech_index;
if ((ii_ > 0) && ((begin_ + (ii_ - 1))->mech_index == tid_)) return;
for (auto i_ = begin_ + ii_; i_ < end_; ++i_) {
if (i_->mech_index != tid_) break;
[[maybe_unused]] auto weight = i_->weight;
unsigned lane_mask_ = arb::gpu::ballot(0xffffffff, tid_<n_);
this was emitted by modcc for the NET_RECV. Note the missing n_
, which I assume should be
end_ - begin_
here?
The different-radii and different-length tests are now uncommented. They should succeed if they are run with this new fix in the diffusion solver: https://github.com/thorstenhater/arbor/commit/7577092f89e9b54d8fde26ac6963a7c9965e43cc
Hey @jlubo,
could you leave them disabled, I'll add them to the to-be-made PR for the fixes.
Done :)
I'll fix the GPU problem in my follow-up #2226
1. It seems like lots of special casing for the different setups (num_sgs=1,2,3). Do you think it would be cleaner to split these into different tests altogether?
I agree, I've now split the function get_morph_and_decor()
into into individual functions for 1, 2, and 3 segments.
2. Question also inline: Can't we always compute the equilibrium concentration?
Yes we can! I've introduced a more general computation of CV volumes making this possible now.
cscs-ci run daint-gpu