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

CINN 代码风格改进计划 #54953

Closed
SigureMo opened this issue Jun 28, 2023 · 18 comments
Closed

CINN 代码风格改进计划 #54953

SigureMo opened this issue Jun 28, 2023 · 18 comments
Assignees
Labels
status/close 已关闭

Comments

@SigureMo
Copy link
Member

SigureMo commented Jun 28, 2023

背景

目前 CINN 已于 #54749 合入 Paddle 主框架,但 CINN 原 repo 的代码风格与 Paddle 不一致,缺少很多用于检查代码风格的工具,因此第一个 PR 只能强行豁免合入,但代码风格的问题应当尽快解决,否则会一直阻塞流水线。

#54749 有三条流水线暴露了代码风格相关问题:

  • PR-CI-APPROVAL log: PR-CI-APPROVAL.log
    image

    主要问题:CINN 侧未检查使用 self.assertTrue,而 Paddle 侧建议使用 np.testing.assert_allclose 来进行替代,详情见 框架易用性提升——单测报错信息优化

  • PR-CI-Codestyle-Check log: PR-CI-Codestyle-Chceck.log

    • 主要问题:pre-commit 中各种 hook 失败,全部为代码风格问题
    • common hooks
      • end-of-file-fixer,直接修复即可
      • trailing-whitespace,直接修复即可
    • Python hooks
      • black,一个 PR 全量格式化即可
      • isort,一个 PR 全量格式化即可
      • flake8,拆分任务
      • ruff,拆分任务
      • pylint,无效 Linter,可忽略
    • C++ hooks
      • clang-format,一个 PR 全量格式化即可
      • cpplint,拆分任务
    • CMake hooks
      • cmake-format,一个 PR 全量格式化即可
      • cmake-lint,拆分任务
  • PR-CI-Static-Check log: PR-CI-Static-Check.log
    image

    • 主要问题:不建议使用 std::coutprint,也许需要和 LOG 改进专项结合(本任务不涉及)

推进方式

Note

本任务需要在 Paddle repo 里修改,而不是在原 CINN repo 里修改

@SigureMo SigureMo assigned SigureMo and unassigned zhangting2020 Jun 28, 2023
@luotao1 luotao1 self-assigned this Jun 28, 2023
@luotao1
Copy link
Contributor

luotao1 commented Jun 28, 2023

任务拆分

✅ 第一阶段 (已完成 🎉)

序号 任务名 认领人 提交 PR
1 blackFormatter CINN 部分全量格式化 @Ainavo #54964
2 isortFormatter CINN 部分全量格式化 @Ainavo #54963
3 clang-formatFormatter CINN 部分全量格式化 @Ainavo #54961
4 cmake-formatFormatter CINN 部分全量格式化 @Difers #54967
5 end-of-file-fixercommon hook CINN 部分全量格式化 @Liyulingyue #54955
6 trailing-whitespacecommon hook CINN 部分全量格式化 @Liyulingyue #54955
7 flake8Linter CINN 部分全量 Exclude 掉 @gouzil #54974
8 ruffLinter CINN 部分全量 Exclude 掉 @gouzil #54974
9 cpplintLinter CINN 部分全量 Exclude 掉 @GreatV #54957
10 cmake-lintLinter CINN 部分全量 Exclude 掉 @ccsuzzh #54978

✅ 第二阶段 (已完成 🎉)

✅ CINN 单测 np.assertTrue 替换

序号 任务名 认领人 提交 PR
11 ✅ 单测 np.assertTrue 替换 @zrr1999 #55038

✅ Flake8 / Ruff 代码风格修复

序号 错误码 错误数量 认领人 提交 PR
- Ruff - pyflakes
12 ✅ F401 364 @Liyulingyue #55012, #55013, #55182
13 ✅ F403 272 @Liyulingyue #55182, #55255
14 ✅ F632 4 @Liyulingyue #55013
15 ✅ F811 2 @Liyulingyue #55013
16 ✅ F821 1 @Liyulingyue #55103
17 ✅ F901 1 @Liyulingyue #55103
- Ruff - flake8-comprehensions
18 ✅ C408 30 @enkilee #55087
19 ✅ C416 1 @enkilee #55087
20 ✅ C417 2 @enkilee #55087
- Ruff - pyupgrade
21 ✅ UP004 6 @gouzil #54988
22 ✅ UP008 4 @gouzil #54988
23 ✅ UP027 5 @gouzil #54988
24 ✅ UP031 2 @gouzil #54988
25 ✅ UP032 29 @gouzil #54988
26 ✅ UP034 1 @gouzil #54988
- Ruff - pylint
27 ✅ PLR0402 26 @gouzil #54990
28 ✅ PLC0414 9 @gouzil #54990
29 ✅ PLE1205 2 @gouzil #54990
- Flake8 - pycodestyle
30 ✅ E265 1 @ccsuzzh #55074
31 ✅ E266 4 @ccsuzzh #55074
32 ✅ E711 4 @ccsuzzh #55074

Note

  • F403 为大存量,无法自动修复的任务,需要分批量、根据情况手动修复(部分目录,比如 python/cinn 应该直接 ignore 掉,不应该修复,看情况吧,这个目录看看好不好整,优先做其他目录,其他目录做完如果这个好整的话,还是建议做一下的)
  • 统计方式为,首先在配置里删除 exclude 掉的 CINN 目录,然后运行 ruff . --statistics / flake8 . --statistics
  • 如需修复某一错误码,需要先手动删除 [CodeStyle][CINN] exclude cinn files in flake8 and ruff config #54974 中 ignore 掉的该错误码,然后运行工具复现问题
  • F403 建议在 F401 全部完成后做,可考虑自动替换,比如 cinn.common 全部成员为 Int, String, Target, Type(举个例子而已,实际肯定比这多),之后一键替换 from cinn.common import *from cinn.common import Int, String, Target, Type,利用 F401 即可自动剪掉无用 imports,全部成员可考虑编译 CINN 直接运行时获取或者直接参考源码(大多数是直接写在 pybind 里的)

✅ CMake Lint 代码风格修复

序号 错误码 错误数量 认领人 提交 PR
33 [readability/logic] 15 @ccsuzzh #54975, #54993

本表单由 @GreatV 统计~

✅ Cpplint 代码风格修复

序号 错误码 错误数量 认领人 提交 PR
34 [runtime/references] 183 @GreatV, @zyfncg #55068, #55009, #55107
35 [readability/casting] 76 @GreatV #55069
36 [whitespace/line_length] 70 @Difers #55020
37 [runtime/explicit] 51 @Difers #55036
38 [readability/braces] 25 @GreatV #55049
39 [runtime/int] 19 @huangjiyi #55006
40 [build/namespaces] 16 @GreatV #55051
41 [runtime/threadsafe_fn] 15 @huangjiyi #55006
42 [readability/todo] 15 @Sahala08 #55022
43 [readability/check] 11 @Sahala08 #55022
44 [runtime/string] 6 @huangjiyi #55006
45 [build/include_order] 6 @enkilee #55085
46 [build/header_guard] 6 @enkilee #55085
47 [build/include_subdir] 5 @enkilee #55085
48 [build/include] 5 @enkilee #55085
49 [whitespace/semicolon] 4 @Liyulingyue #55046
50 [whitespace/empty_if_body] 3 @gouzil #55021
51 [whitespace/comments] 2 @gouzil #55021
52 [whitespace/blank_line] 2 @gouzil #55021
53 [runtime/init] 1 @gouzil #55021
54 [runtime/casting] 1 @gouzil #55021
55 [build/storage_class] 1 @gouzil #55021

本表单由 @GreatV 统计~

Note

  • 可参考 [CodeStyle][CINN] fix cinn cpplint codestyle #55006,先注释掉 .pre-commit-hooks.yaml 中的 cpplint exclude,之后运行 pre-commit run cpplint-cpp-source --all-files | grep "runtime/int" 来查找自己认领的错误码
  • [readability/todo] 这类可以考虑在原 repo https://github.com/PaddlePaddle/CINN 找到原作者,并在 TODO 后填写 GitHub id
  • 框架内其他地方仍遗留一些 cpplint 问题,本任务不涉及非 CINN 部分,当然如果有能力可以一起修改

@Ainavo
Copy link
Contributor

Ainavo commented Jun 28, 2023

1,2, 3

@Liyulingyue
Copy link
Contributor

5 6

@zrr1999
Copy link
Member

zrr1999 commented Jun 28, 2023

11

@GreatV
Copy link
Contributor

GreatV commented Jun 28, 2023

统计信息已更新至表单~

Cpplint 代码风格修复 Cpplint 代码风格修复
序号 错误码 错误数量 认领人 提交 PR
1 [whitespace/line_length] 12776 clang-format格式化
2 [runtime/references] 206
3 [readability/casting] 79
4 [runtime/explicit] 65
5 [readability/braces] 27
6 [runtime/int] 19
7 [readability/todo] 16
8 [build/namespaces] 16
9 [runtime/threadsafe_fn] 15
10 [readability/check] 11
11 [build/include] 9
12 [build/header_guard] 8
13 [whitespace/ending_newline] 6
14 [runtime/string] 6
15 [build/include_order] 6
16 [build/include_subdir] 5
17 [whitespace/semicolon] 4
18 [whitespace/empty_if_body] 3
19 [whitespace/blank_line] 3
20 [whitespace/comments] 2
21 [readability/namespace] 2
22 [runtime/init] 1
23 [runtime/indentation_namespace] 1
24 [runtime/casting] 1
25 [runtime/arrays] 1
26 [readability/inheritance] 1
27 [legal/copyright] 1
28 [build/storage_class] 1

@GreatV
Copy link
Contributor

GreatV commented Jun 28, 2023

统计信息已更新至表单~

CMake Lint 代码风格修复
序号 文件名 错误码 错误数量 认领人 提交 PR
1 paddle/cinn/auto_schedule/
search_strategy/mutate_rule/CMakeLists.txt
[whitespace/mismatch]
[whitespace/tabs]
2 cmake-format格式化
2 cmake/cinn/core.cmake [readability/logic] 15
3 paddle/cinn/auto_schedule/
search_space/auto_gen_rule/CMakeLists.txt
[whitespace/tabs] 5 cmake-format格式化
4 paddle/cinn/auto_schedule/
post_schedule_rule/CMakeLists.txt
[whitespace/mismatch]
[whitespace/tabs]
2 cmake-format格式化

@gouzil
Copy link
Member

gouzil commented Jun 29, 2023

21-26

@gouzil
Copy link
Member

gouzil commented Jun 29, 2023

27-29

@GreatV
Copy link
Contributor

GreatV commented Jun 29, 2023

统计信息已更新至表单~

Update: Cpplint 代码风格修复
序号 错误码 错误数量 认领人 提交 PR
1 [runtime/references] 183
2 [readability/casting] 76
3 [whitespace/line_length] 70
4 [runtime/explicit] 51
5 [readability/braces] 25
6 [runtime/int] 19
7 [build/namespaces] 16
8 [runtime/threadsafe_fn] 15
9 [readability/todo] 15
10 [readability/check] 11
11 [runtime/string] 6
12 [build/include_order] 6
13 [build/header_guard] 6
14 [build/include_subdir] 5
15 [build/include] 5
16 [whitespace/semicolon] 4
17 [whitespace/empty_if_body] 3
18 [whitespace/comments] 2
19 [whitespace/blank_line] 2
20 [runtime/init] 1
21 [runtime/casting] 1
22 [build/storage_class] 1

@Sahala08
Copy link
Contributor

42, 43

@enkilee
Copy link
Contributor

enkilee commented Jun 29, 2023

45 46 47 48

@Difers
Copy link
Contributor

Difers commented Jun 29, 2023

36, 37

@Liyulingyue
Copy link
Contributor

49

@huangjiyi
Copy link
Member

39, 41, 44

@ccsuzzh
Copy link
Contributor

ccsuzzh commented Jun 29, 2023

认领 30,31,32

@luotao1
Copy link
Contributor

luotao1 commented Jul 11, 2023

CINN 代码风格改进计划 已全部完成,感谢参与的小伙伴们

@Ainavo @Difers @Liyulingyue @gouzil @GreatV @ccsuzzh @zrr1999 @enkilee @ccsuzzh @huangjiyi @Sahala08

欢迎继续参与快乐开源的其他任务

@luotao1 luotao1 closed this as completed Jul 11, 2023
@paddle-bot paddle-bot bot added the status/close 已关闭 label Jul 11, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Call for Contributions Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/close 已关闭
Projects
Development

No branches or pull requests