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

99 stream put uses inconsistent stream #111

Merged
merged 6 commits into from
Oct 21, 2022

Conversation

quetric
Copy link
Collaborator

@quetric quetric commented Oct 19, 2022

No description provided.

quetric and others added 3 commits July 27, 2022 13:40
* Hotfix issue with storing aligned buffer

* Add missing include

Co-authored-by: Tristan Laan <tristan.laan@gmail.com>
@quetric quetric requested a review from Mellich October 19, 2022 12:49
@quetric quetric marked this pull request as ready for review October 19, 2022 12:50
@Mellich
Copy link
Contributor

Mellich commented Oct 19, 2022

Every test seems to work and looks good to have a zero-based stream ID range for the API now. But couldn't this lead to problems when using the stream IDs in user kernels?

For example in this test. I changed the send/recv into a stream_put and now the received stream ID will be different from the one provided in the stream_put so the test will hang.

Copy link
Contributor

@Mellich Mellich left a comment

Choose a reason for hiding this comment

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

See comment above

@quetric quetric requested a review from Mellich October 20, 2022 15:21
@Mellich
Copy link
Contributor

Mellich commented Oct 21, 2022

Works now!

Maybe we could also add the change of the loopback test to this PR, so we also have a test that proofs that the stream IDs work as expected?
Would be this commit 6e9ae6d.

@Mellich
Copy link
Contributor

Mellich commented Oct 21, 2022

I rebased #94 on this PR and tried to run the stream_put from to same destination rank. The stream ID does still not match in this case.
See b5ee661 test_loopback_local_res in HLS tests.

@Mellich Mellich merged commit 077bd10 into dev Oct 21, 2022
@quetric quetric deleted the 99-stream_put-uses-inconsistent-stream_id branch October 28, 2022 16:32
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