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

Fix TestDestroy.TestDestroyTensorSingle #279

Merged

Conversation

photon-schiesser
Copy link
Contributor

This PR fixes this test by first syncing the tensor data to the device. When I run the test suite multiple times on my real hardware, the memory on the device was never initialized, so the test would fail on the first run, then succeed, then fail thereafter.

…ata to the device

Signed-off-by: ScheissSchiesser <9285100+ScheissSchiesser@users.noreply.github.com>
@photon-schiesser photon-schiesser force-pushed the fix_testDestroyTensorSingle branch from 4c546f6 to 409c455 Compare April 9, 2022 15:43
Copy link
Member

@axsaucedo axsaucedo left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution - looks great. I just added a comment mentioning you don't need the SyncLocal at the top, if you remove that it should be good to merge.

test/TestDestroy.cpp Show resolved Hide resolved
test/TestDestroy.cpp Outdated Show resolved Hide resolved
test/TestDestroy.cpp Show resolved Hide resolved
test/TestDestroy.cpp Show resolved Hide resolved
Signed-off-by: ScheissSchiesser <9285100+ScheissSchiesser@users.noreply.github.com>
@photon-schiesser photon-schiesser force-pushed the fix_testDestroyTensorSingle branch from 65be60f to 54baa18 Compare April 12, 2022 04:32
@axsaucedo
Copy link
Member

NIce one @ScheissSchiesser - thank you for the contribution

@axsaucedo axsaucedo merged commit 8b08e7c into KomputeProject:master Apr 12, 2022
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.

2 participants