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 F# sample #1105

Closed
wants to merge 1 commit into from
Closed

Fix F# sample #1105

wants to merge 1 commit into from

Conversation

kant2002
Copy link
Contributor

@kant2002 kant2002 commented Oct 7, 2023

No description provided.

@kant2002
Copy link
Contributor Author

kant2002 commented Oct 7, 2023

Since I did not touch license headers, I would like to understand what's my future steps here.

@m4rs-mt m4rs-mt added this to the v2.0 milestone Oct 7, 2023
@MoFtZ
Copy link
Collaborator

MoFtZ commented Oct 9, 2023

hi @kant2002. Thank you for taking an interested in ILGPU.

Looking at your PR, what problem is it trying to fix in the F# sample code?

@kant2002
Copy link
Contributor Author

When I run sample code the GPU part return 0 instead of results

@MoFtZ
Copy link
Collaborator

MoFtZ commented Oct 10, 2023

@kant2002 That actually sounds like a bug with ILGPU, not with the sample project.

The call to GetAsArray1D internally calls Synchronize. It should not be necessary to call Synchronize twice (once explicitly, and a second time within GetAsArray1D).

@MoFtZ
Copy link
Collaborator

MoFtZ commented Oct 10, 2023

@kant2002 OK, I see the problem now.

The F# sample project is launching the kernel on the default stream, rather than the newly created stream. I have raised #1106 to fix this issue.

@m4rs-mt
Copy link
Owner

m4rs-mt commented Oct 10, 2023

@kant2002 welcome to the ILGPU community and thank you for raising this PR. Nice catch 👍!

@m4rs-mt
Copy link
Owner

m4rs-mt commented Oct 18, 2023

This patch has been merged in #1106.

@m4rs-mt m4rs-mt closed this Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants