Support next brian release

[denisalevi]

This ended up being a development branch where I merged all updates from brian2 master, which came after the last release (2.4.2). Will be merged with the next brian2 release.

My update procedure: I first go through all Brian2 PRs and try to filter out those that changed something that required an update in Brian2CUDA. Then I go through each selected PR in their merge order and do the following:

1. Update Brian2 in the frozen_repos/brian2 submodule (see e.g. f0b8d54)

   cd frozen_repos/brian2
   # remove Brian2 patch
   git stash
   git checkout <brian2-PR-merge-commit>
   cd ..
   git add brian2
   git commit -m "Update Brian2 submodule to PR brian-team/brian2#<PR-number>

2. Apply the Brian2 patch and solve problems if it can't apply correctly

   cd brian2  # frozen_repos/brian2
   git apply ../brian2.diff
   # if this applied without problems, you are done here and can continue with 3.
   # if the patch didn't apply, fix it by applying the changes via git stash pop and fixing the conflicts
   git stash pop
   # fix the conflicts if there are any
   # remove any changes from git index (we don't want to commit anything to Brian2)
   git reset
   # update the patch file
   git diff > ../brian2.diff
   # commit the patch file to brian2cuda (if it changed at all)
   cd ..
   git add brian2.diff
   git commit -m "Update `brian2.diff` after updating Brian2 to PR brian-team/brian2#<PR-number>"

3. Make any changes in Brian2CUDA if they are required after the Brian2 update

[mstimberg]

I struggled a bit with the merge, but after that everything went smoothly. Except for

+        print("INFO _last_run_time = {} s".format(self._last_run_time))

the brian2.diff patch now only includes changes to feature/speed tests.

I'm getting two failed tests. The first test failure is test_semantics_floor_division, since it gets an unexpected warning about "Detected code statement with default function and 64bit integer type in the same line.". I'm not 100% sure whether we should make the Brian test more lenient for that, or whether this is something we can change in brian2cuda. If I am not mistaken, brian2cuda's check for a _brian_floor function gets triggered by _brian_floordiv and I'm not quite sure whether this function is affected by the issue. If not, adding [(] to the end of the regex might do the trick. And of course we'd have to remove the test in Brian2CUDA's test suite that expects one warning :)

The second test that fails is test_no_gpu_detection_preference which fails with

ERROR: cudaMemcpyToSymbol(d_curand_seed, &dev_curand_seed, sizeof(unsigned long long*)) failed at objects.cu:107 : invalid device symbol

But I just realized that this is probably simply due to the fact that the test sets compute capability to 6.1 while the ancient GPU I use on my work machine has only compute capability 5.0 😆

So I guess after dealing with the floor division warning we are ready to merge this PR (after adapting install_requires in setup.py)?

[denisalevi]

Hey @mstimberg , thanks for this! I will have a look at it tomorrow afternoon and run the test suite in the meantime on my GPU :)

Could probably make test_no_gpu_detection_preference dependent on detected GPU... Will have a look.

And for the warning, I'll also look into it. I have to remember what the issue was, but I came across the problem before, and there is also an issue for potentially getting rid of that warning altogether... #135

[mstimberg]

Hey @mstimberg , thanks for this! I will have a look at it tomorrow afternoon and run the test suite in the meantime on my GPU :)

👍

Could probably make test_no_gpu_detection_preference dependent on detected GPU... Will have a look.

I would not worry too much about it – maybe simply set it to the lowest supported compute capability?

[denisalevi]

Alright, i had a rough look over the changes, looks all good to me. If all tests are passing, it must be right, right? :P

Super nice that brian2.diff is now only required for benchmarks! And that part has changed so much compared to the Brian2 version that its probably best to copy it over to Brian2CUDA anyways. Well, next time we want to run benchmarks maybe :)

The test_semantics_floor_division issue I already fixed in the past btw. There is a Brian2CUDA version of the test that expects the warning and the Brian2 test is ignored when running brian2cuda.test(), see here. I just didn't make the change in the test suite script in the tools directory yet. The tools directory test should just use the brian2cuda.test() function, but if I remember correctly the change needed some additional work that I didn't get around to do yet. So maybe running brian2cuda.test() is actually the better way to test if the tests pass :).

And I updated test_no_gpu_detection_preference to use the minimal supported compute capability.

Great, merging this!

[denisalevi]

I just realized that the all the changes in brian2.diff coming below here have changed with this PR? This broke our benchmark scripts. I'm opening an issue for this.

@mstimberg Do you have an idea what happened? Since you seem to have pushed some changes here, but not the ones we had before? Could it be that you had an outdated brian2.diff to start with? Maybe it wasn't updated by the merge from master?

[mstimberg]

Um, no sorry, not quite sure what happened here. But I had a hard time looking making proper sense of diffs of diffs, so it's entirely possible that I messed this up...