Closed ptheywood closed 1 year ago
Changes look reasonable.
Can test on Windows on my office machine if you think necessary.
I think it should be fine, or atleast no worse than the current implementation in that case. I.e. I'm confident in the CUDA 12 behaviour with this patch, I'm less confident about the existing CUDA 11 behaviour.
So I'd lean towards merging this in, and then maybe we should just check it on a windows multi-gpu at some point when convinient anyway, as I doubt we've ran FLAMEGPU2 on a multi-gpu box in windows for quite a while (Unless @mondus' machine has 2 GPUs in currently, rather than just a 3080?).
Yeah my office machine has the 750ti and titanx on Windows, hence my suggestion of that if we care.
When using CUDA 12.x in a multi-gpu system, an interaction between
flamegpu::cleanup()
andcudaDestroyStreams
during~CUDASimulation()
was casuing exceptions to be thrown.CUcontext
can/will return the same handle after a reset, so we cannot "remember" the context that way, but CUDA 12 introduces a method which allows us to compare a uniqueunsigned long long int
which can be used to check the context is a match.No new tests are required, the existing cleanup tests triggered the issue with multi-gpu + CUDA 12.
Closes #1053
Tested in a multi-gpu linux system with CUDA 11.0, 11.8 and 12.0 with different
CUDA_VISIBLE_DEVICES
values and on a single GPU system with CUDA 12.0.