-
Notifications
You must be signed in to change notification settings - Fork 45
feat: enforce Google-based clang-format style and add style checker #156
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -217,6 +217,11 @@ if(BUILD_TEST) | |
| add_subdirectory(tests) | ||
| endif() | ||
|
|
||
| if(USE_MACA) | ||
| add_executable(test_maca_allocator test/runtime/test_maca_allocator.cc) | ||
| link_infini_train_exe(test_maca_allocator) | ||
| endif() | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 去掉。
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| # Negative compile test: missing dtype registration must fail at compile time. | ||
| set(DTYPE_DISPATCH_COMPILE_FAIL_SOURCE | ||
| ${PROJECT_SOURCE_DIR}/tests/dtype/test_dtype_dispatch_compile_fail.cc) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| #include "infini_train/include/autograd/grad_mode.h" | ||
|
|
||
| namespace infini_train::autograd { | ||
| thread_local bool GradMode::grad_enabled_ = true; | ||
| thread_local bool GradMode::tls_grad_enabled_ = true; | ||
| } // namespace infini_train::autograd |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| #include "infini_train/src/core/ccl/cuda/nccl_impl.h" | ||
|
|
||
| #include <nccl.h> | ||
| #include <vector> | ||
|
|
||
| #include <nccl.h> | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. c 语言系统库头文件应当放在 c++标准库头文件上一个分组。
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NCCL 是第三方通信库,不是 c 语言系统库吧 |
||
|
|
||
| #include "glog/logging.h" | ||
|
|
||
| #include "infini_train/include/common/cuda/common_cuda.h" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| #include "infini_train/include/autograd/linear.h" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这个不是配套头文件,不需要放首行,以及理论上 kernel 层不应当引用 autograd 层头文件。
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这里不需要引用这个头文件,删掉了错误引用及添加规则。但目前头文件的匹配检查机制还是根据后缀来的,是clang-format默认行为。 |
||
|
|
||
| #include <cstdint> | ||
| #include <memory> | ||
| #include <numeric> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个文件不要动。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我感觉其他不动,关于头文件顺序检查可以保留。但我看头文件检测也都是简单的按规则匹配,有可能出现错误(文件名相同但并不是对应头文件)或后续需要更新(比如添加third_party)。