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

[opengl] random use different seed in each launch #692

Merged
merged 3 commits into from
Apr 2, 2020

Conversation

archibate
Copy link
Collaborator

So that examples/sdf_renderer.py works.

Related issue = #492

[Click here for the format server]

@yuanming-hu
Copy link
Member

Thanks! I'll test it on my end. Meanwhile, let's finish #666 first so that we get rid of the redundant commits.

@archibate archibate requested a review from yuanming-hu April 2, 2020 15:38
@archibate
Copy link
Collaborator Author

We may also want to add a test case for this, to check if ti.random() return same when called again. e.g.

@ti.kernel
def func(i: ti.i32):
  x[i] = ti.random()
for i in range(5):
  func(i)
  assert x[i] != x[i - 1]

@yuanming-hu
Copy link
Member

We may also want to add a test case for this, to check if ti.random() return same when called again. e.g.

@ti.kernel
def func(i: ti.i32):
  x[i] = ti.random()
for i in range(5):
  func(i)
  assert x[i] != x[i - 1]

This might be a brittle test since there's a tiny probability that x[i] == x[i - 1]. It will be better to, say repeat 5 times and assert there are at least 4 different results.

[skip ci] fix access out of bound in test_random
@yuanming-hu
Copy link
Member

Screenshot from 2020-04-02 12-00-36

I confirm this works on my end. It reaches 90 samples per pixel per second, which is 1.8x faster than the CUDA backend!

@archibate
Copy link
Collaborator Author

archibate commented Apr 2, 2020

That's amazing! Couldn't believe we are now faster than CUDA! Does this mean that CUDA still has lot of space of optimizations to be done? Or simply OpenGL is really better than CUDA?

@yuanming-hu
Copy link
Member

I guess it's because CUDA is not yet fully optimized (and the OpenGL backend is well done). I haven't got a chance to work on it.

@archibate
Copy link
Collaborator Author

Seems #603 is just 杞人忧天ing... the performance of current OpenGL backend is really utilized now.
NEXT: to support sparse data structures on GL, so that the thought-to-be-weak GLSL can really 平起平坐 with CUDA :)

@archibate
Copy link
Collaborator Author

#603 says:

Most time costed on sync (data transfer from&to GPU) thanks to GL's great API design, we may want to figure out how to use it more efficiently.

In fact I found sync is only costly on mesa one, not NV, maybe their GPUs have really faster communication with CPU?

@yuanming-hu
Copy link
Member

#603 says:

Most time costed on sync (data transfer from&to GPU) thanks to GL's great API design, we may want to figure out how to use it more efficiently.

In fact I found sync is only costly on mesa one, not NV, maybe their GPUs have really faster communication with CPU?

I guess it's because the NVIDIA driver is better implemented than mesa for NVIDIA GPUs.

@archibate
Copy link
Collaborator Author

Could you also test examples/nbody_oscillator.py on NV-GL & CUDA? I guess CUDA is better when dealing with memory intensive kernels (doesn't it?)

@yuanming-hu
Copy link
Member

I have to change N to 80000 to make a difference. Then OpenGL reaches 15 FPS and CUDA 5 FPS. OpenGL is 3x faster :-) It seems me that there's a performance bug in the CUDA backend, which I should fix as soon as possible...

@yuanming-hu
Copy link
Member

I updated https://github.com/taichi-dev/taichi/pull/692/files#diff-733c00d9e85a14e095b788b8b985e75c

Btw do you want to fix the memory leakage here? If not I'll go merge this PR.

@archibate
Copy link
Collaborator Author

It seems me that there's a performance bug in the CUDA backend, which I should fix as soon as possible...

Sounds a serious issue, good luck!

Btw do you want to fix the memory leakage here? If not I'll go merge this PR.

No, let's just keep this PR small and trackable :)

@yuanming-hu
Copy link
Member

Cool, thanks. Merging this in now...

@yuanming-hu yuanming-hu merged commit 7b36dda into taichi-dev:master Apr 2, 2020
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