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

Basic synchronization support with sync() #642

Merged
merged 1 commit into from
May 31, 2024
Merged

Conversation

aayushg55
Copy link
Contributor

@aayushg55 aayushg55 commented May 31, 2024

  • Added synchronization support through sync() for both host and cuda executors
  • Updated all tests to use calls to sync() instead of cudaStreamSynchronize() or cudaDeviceSynchronize()
  • Added sync() to the documentation
  • Removed calls to Prefetch

Other:

  • Fix to Upsample test in OperatorTests.cu to sync before comparison and catch errors
  • Fix to CmakeLists.txt file to copy test.npy for an 00_io test

@aayushg55 aayushg55 requested a review from cliffburdick May 31, 2024 00:52
@cliffburdick
Copy link
Collaborator

/blossom-ci

Copy link
Collaborator

@cliffburdick cliffburdick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I noticed you fixed several bugs too. Just a few small comments. Please rebase those into a single commit once you update and we can push this through CI

@cliffburdick
Copy link
Collaborator

/blossom-ci

Updated all tests to use calls to .sync() instead of the cuda api and
removed Prefetch calls.
@cliffburdick
Copy link
Collaborator

/blossom-ci

@coveralls
Copy link

Coverage Status

coverage: 92.666% (-0.9%) from 93.592%
when pulling db2ae6b on cpu_executor
into 1e24924 on main.

@cliffburdick cliffburdick merged commit 381a6b2 into main May 31, 2024
1 check passed
@cliffburdick
Copy link
Collaborator

/blossom-ci

@cliffburdick cliffburdick deleted the cpu_executor branch May 31, 2024 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants