Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GPU buffers will never bypass the ADIOS internal buffers #3410

Merged
merged 2 commits into from Dec 16, 2022

Conversation

anagainaru
Copy link
Contributor

The BP5 tests do not pass for contiguous buffers because the IsContiguousTransfer function returns true regardless of the buffer memory space. The bug appears after PR #3387.
@eisenhauer you mention that you made sure to avoid the case of GPU buffers but I cannot find any code for this. Maybe it was removed later, but in the current form, all the GPU tests with BP5 are failing.

@eisenhauer
Copy link
Member

Yes, in some interim version of the code I did exclude based on the memspace destination, but that must have gotten dropped. My only concern here is that either this check shouldn't be in IsContiguousTransfer() (because it's a check on something beside contiguity), or that routine should be renamed. I think previously I had it outside, setting the DirectToAppMemory flag to false if the destination memory was in CUDA memspace.

@anagainaru
Copy link
Contributor Author

@eisenhauer Makes sense, I moved it outside as I assume you had it before.
Another option is to rename the function to IsContiguousCPUTransfer but it doesn't read too well. And if we want to use GPUdirect at any point we will have to change the function again. I'll make sure if anyone uses this function it checks the memory space first.

@anagainaru anagainaru changed the title Excluding GPU buffers from bypassing the ADIOS internal buffers GPU buffers will never bypass the ADIOS internal buffers Dec 16, 2022
@anagainaru anagainaru merged commit 580f9ab into ornladios:master Dec 16, 2022
@anagainaru anagainaru deleted the bug-cuda-bp5 branch December 27, 2022 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants