Tests for particle diffusion

[jlubo]

Python tests are added to ensure the working of the diffusion of arbitrary particles in setups with 1-3 segments (cf. issues #2084 and #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 #2145 has been resolved.

[thorstenhater]

Hey @jlubo,

thanks for adding this. Could you run black . and push the results? That'll make the Linter
check pass.

[thorstenhater]

And flake8 . :D

[jlubo]

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 :)

[thorstenhater]

Hey,
made some comments for improvements, rest looks great, thanks.

[jlubo]

Hey, made some comments for improvements, rest looks great, thanks.

Thanks @thorstenhater! I've revised the code according to the comments.

[thorstenhater]

Thank you very much!

[thorstenhater]

cscs-ci run daint-gpu

[thorstenhater]

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?

[jlubo]

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: thorstenhater@7577092

[thorstenhater]

Hey @jlubo,

could you leave them disabled, I'll add them to the to-be-made PR for the fixes.

[jlubo]

Done :)

[thorstenhater]

I'll fix the GPU problem in my follow-up #2226

[thorstenhater]

Nice! See also downstream #2226. Two questions:

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?
2. Question also inline: Can't we always compute the equilibrium concentration?

[jlubo]

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.

[boeschf]

cscs-ci run daint-gpu

[thorstenhater]

Hey @jlubo,

could I close this? It's subsumed in #2226.

[jlubo]

Yes, sure. I'll check #2226 again in the next days.