OpenGL backend (stage 1) #535

archibate · 2020-02-25T15:18:34Z

Related issue id = #492

archibate · 2020-02-25T15:19:13Z

This is a fragment of #495.

yuanming-hu

Thanks! Looks good in general except for a few places to be changed slightly.

cmake/TaichiCore.cmake

taichi/platform/opengl/opengl_data_types.h

taichi/backends/codegen_opengl.cpp

taichi/program.cpp

k-ye · 2020-02-26T09:10:12Z

I found a styling issue. Taichi codebase follows this

void foo() {
  // ...
}

instead of starting { at a new line... Before merging, could you run the formatter to fix those :)?

cmake/TaichiCore.cmake

examples/opengl_backend.py

yuanming-hu · 2020-02-26T09:20:33Z

Before merging, could you run the formatter to fix those :)?

The formatter we use is clang-format-6.0. Note that different versions of clang-format gives you slightly different results. Sorry, I didn't make this explicit in the developer guideline. Btw, is there a code format checker service for github? Maybe we can add that to the ckecks, just like Travis amd AppVeyor.

yuanming-hu · 2020-02-26T09:23:53Z

Sooner or later we may want to introduce something like cpplint to Taichi so that some bugs can be detected early. Not sure if there's a way to set that up just like Travis. I've might have seen other projects using related services though.

taichi/platform/opengl/opengl_kernel.h

k-ye · 2020-02-26T09:31:49Z

The formatter we use is clang-format-6.0. Note that different versions of clang-format gives you slightly different results.

Hmm, I thought the styling is determined by clang_format_style, e.g. LLVM or Google?

Sooner or later we may want to introduce something like cpplint to Taichi

I see. TBH I don't know much about the CI infra. But can we add a run-linter step? And if that step fails, it means the code doesn't comply with Taichi's coding style, so the authors should format their code before merging?

yuanming-hu · 2020-02-26T09:39:02Z

Hmm, I thought the styling is determined by clang_format_style, e.g. LLVM or Google?

Ideally, yes. In practice, I find the version of clang-format also affects the result. So it's (clang-format-version + .clang-format) that determines the final code format :-)

I see. TBH I don't know much about the CI infra. But can we add a run-linter step? And if that step fails, it means the code doesn't comply with Taichi's coding style, so the authors should format their code before merging?

Yeah we used to have a githook for running clang-format. Not sure if we should re-enable that. The downside is that with these enabled it would take a couple more seconds for git commit and not every developer would take the time to install the linters.

Returning to bed now and happy to talk more about code format issues tmr. (I was getting up to use the bathroom yet for some reason ended up with coding... LOL)

k-ye · 2020-02-26T09:47:51Z

I see. I guess we can enforce formatting when the project becomes more mature... (And sorry about discussing this here @archibate , this should have been discussed this in #466 ... )

Returning to bed now and happy to talk more about code format issues tmr.

LOL, I thought you travelled to another time zone

archibate · 2020-02-26T10:51:12Z

I found a styling issue. Taichi codebase follows this
void foo() {
  // ...
}
instead of starting { at a new line... Before merging, could you run the formatter to fix those :)?

Oh! I see... I think we'd better run ti format after all stages done instead of now, so that git merge won't bother me with conflicts :)

archibate · 2020-02-26T14:49:33Z

worker 'gw0' crashed while running 'tests/python/test_ad_atomic.py::test_ad_reduce'
================== 1 failed, 251 passed in 453.86s (0:07:33) ==================

Why is pytest always crashing these days...

k-ye · 2020-02-26T23:41:16Z

Why is pytest always crashing these days...

Actually, is it a legit usage to have multiple OpenGL context in multiple threads? I somehow remembered this singleton design of OpenGL, could that be the root cause...?

archibate · 2020-02-27T03:30:14Z

Actually, is it a legit usage to have multiple OpenGL context in multiple threads? I somehow remembered this singleton design of OpenGL, could that be the root cause...?

This happens in other PRs too, where GL codes are not merged at all.

archibate · 2020-02-27T04:07:59Z

I believe #523 will solve this after its merge.

yuanming-hu · 2020-02-27T05:34:43Z

I see. I guess we can enforce formatting when the project becomes more mature... (And sorry about discussing this here @archibate , this should have been discussed this in #466 ... )

I think we'd better run ti format after all stages done instead of now, so that git merge won't bother me with conflicts :)

Yeah, right before we release v0.6.0, let's reformat everything, and enforce every future commit follow certain code format.

yuanming-hu

I've done one more pass. I think this is ready to squash & merge after the potential memory leakages are fixed. What do you think @k-ye?

taichi/platform/opengl/opengl_api.cpp

[skip ci] test inout data[3] success [skip ci] try to pass kernel arguments

fix bug x[4] -> x[4 >> 2] = x[1]

remove opengl from all_archs to prevent test

optional TI_WITH_OPENGL ON (detect auto OFF) TI_WITH_OPENGL guard opengl_api.cpp fix typo

Update profiler.cpp

TI_WITH_OPENGL=OFF by default TI_WITH_OPENGL default to ON

k-ye

I think i only have a few nits, otherwise LGTM! (I don't have much experience with OpenGL, but i think in general it looks good )

python/taichi/lang/__init__.py

taichi/backends/struct_opengl.cpp

taichi/program.cpp

archibate · 2020-02-27T09:27:13Z

Looks good. Preparing g2 now.

fix typo

archibate · 2020-02-27T16:18:13Z

Anybody?

yuanming-hu · 2020-02-27T16:34:55Z

Sorry I have a few meetings this morning. I think this PR looks good now. Merging! Thank you again for this amazing piece of contribution!!

archibate requested review from yuanming-hu and k-ye and removed request for yuanming-hu February 25, 2020 15:19

yuanming-hu requested changes Feb 26, 2020

View reviewed changes

cmake/TaichiCore.cmake Outdated Show resolved Hide resolved

taichi/platform/opengl/opengl_data_types.h Outdated Show resolved Hide resolved

archibate force-pushed the g1 branch from e793657 to 1d231d6 Compare February 26, 2020 07:29

archibate requested a review from yuanming-hu February 26, 2020 07:34

archibate force-pushed the g1 branch from 0433548 to b59e047 Compare February 26, 2020 08:44

k-ye reviewed Feb 26, 2020

View reviewed changes

taichi/backends/codegen_opengl.cpp Show resolved Hide resolved

k-ye reviewed Feb 26, 2020

View reviewed changes

taichi/program.cpp Outdated Show resolved Hide resolved

yuanming-hu requested changes Feb 26, 2020

View reviewed changes

cmake/TaichiCore.cmake Outdated Show resolved Hide resolved

examples/opengl_backend.py Outdated Show resolved Hide resolved

yuanming-hu reviewed Feb 26, 2020

View reviewed changes

taichi/platform/opengl/opengl_kernel.h Outdated Show resolved Hide resolved

archibate requested review from yuanming-hu and k-ye February 26, 2020 13:18

yuanming-hu requested changes Feb 27, 2020

View reviewed changes

taichi/platform/opengl/opengl_api.cpp Outdated Show resolved Hide resolved

taichi/platform/opengl/opengl_api.cpp Outdated Show resolved Hide resolved

taichi/platform/opengl/opengl_api.cpp Outdated Show resolved Hide resolved

archibate added 2 commits February 27, 2020 15:22

[skip ci] basic start up for opengl

b1d27b9

[skip ci] test inout data[3] success [skip ci] try to pass kernel arguments

[skip ci] use a struct_metal style struct compiler

f4e7fe5

archibate and others added 13 commits February 27, 2020 15:22

[skip ci] basic allocator

1f79fc1

[skip ci] basic read/write tensor

ef922ce

fix bug x[4] -> x[4 >> 2] = x[1]

adding f32&f64 support for opengl

b7594bd

remove opengl from all_archs to prevent test

Update TaichiCore.cmake for TI_WITH_OPENGL

ce00545

optional TI_WITH_OPENGL ON (detect auto OFF) TI_WITH_OPENGL guard opengl_api.cpp fix typo

Update test_abs.py

9e68286

merge g1 with master fix: x86_64 -> x64

aeb2543

Update profiler.cpp

replace opengl_unary_op_type_symbol with unary_op_type_name

ea41c2a

snode_io_arch -> snode_accessor_arch

b2acaa9

use GLEW_LIBRARY_DIR

2ebf348

TI_WITH_OPENGL=OFF by default TI_WITH_OPENGL default to ON

no more USE_GLEW macro

2132928

no more mallocs in SSBO

d8457b5

remove opengl_backend.py, add comment in mpm99 for ti.opengl

4adaaac

fix memory leakage using std::vector

c22ca77

archibate force-pushed the g1 branch from 4a3fa91 to c22ca77 Compare February 27, 2020 07:24

archibate requested a review from yuanming-hu February 27, 2020 07:25

k-ye reviewed Feb 27, 2020

View reviewed changes

python/taichi/lang/__init__.py Outdated Show resolved Hide resolved

taichi/backends/struct_opengl.cpp Show resolved Hide resolved

taichi/program.cpp Show resolved Hide resolved

archibate added 3 commits February 27, 2020 17:14

better cmake: find GLFW and glfw3

99725de

merge g1 with master

724781c

support arch: uncomment with_opengl

4ee3102

archibate requested a review from k-ye February 27, 2020 14:05

initialize_opengl in materialize_layout

b0a8886

fix typo

archibate force-pushed the g1 branch from 4174113 to b0a8886 Compare February 27, 2020 14:36

yuanming-hu approved these changes Feb 27, 2020

View reviewed changes

yuanming-hu merged commit dd6da1f into taichi-dev:master Feb 27, 2020

archibate mentioned this pull request Feb 27, 2020

(MERGE ME) emergency fix for non-GL build (1) #546

Merged

archibate mentioned this pull request May 4, 2020

[IR] Use JIT compilation/evaluation for systematic constant folding #839

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OpenGL backend (stage 1) #535

OpenGL backend (stage 1) #535

archibate commented Feb 25, 2020

archibate commented Feb 25, 2020

yuanming-hu left a comment

k-ye commented Feb 26, 2020

yuanming-hu commented Feb 26, 2020

yuanming-hu commented Feb 26, 2020

k-ye commented Feb 26, 2020

yuanming-hu commented Feb 26, 2020

k-ye commented Feb 26, 2020

archibate commented Feb 26, 2020 •

edited

Loading

archibate commented Feb 26, 2020

k-ye commented Feb 26, 2020

archibate commented Feb 27, 2020

archibate commented Feb 27, 2020

yuanming-hu commented Feb 27, 2020

yuanming-hu left a comment

k-ye left a comment

archibate commented Feb 27, 2020

archibate commented Feb 27, 2020

yuanming-hu commented Feb 27, 2020

OpenGL backend (stage 1) #535

OpenGL backend (stage 1) #535

Conversation

archibate commented Feb 25, 2020

archibate commented Feb 25, 2020

yuanming-hu left a comment

Choose a reason for hiding this comment

k-ye commented Feb 26, 2020

yuanming-hu commented Feb 26, 2020

yuanming-hu commented Feb 26, 2020

k-ye commented Feb 26, 2020

yuanming-hu commented Feb 26, 2020

k-ye commented Feb 26, 2020

archibate commented Feb 26, 2020 • edited Loading

archibate commented Feb 26, 2020

k-ye commented Feb 26, 2020

archibate commented Feb 27, 2020

archibate commented Feb 27, 2020

yuanming-hu commented Feb 27, 2020

yuanming-hu left a comment

Choose a reason for hiding this comment

k-ye left a comment

Choose a reason for hiding this comment

archibate commented Feb 27, 2020

archibate commented Feb 27, 2020

yuanming-hu commented Feb 27, 2020

archibate commented Feb 26, 2020 •

edited

Loading