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

[Test Mv] quantization #51942

Merged
merged 9 commits into from
Mar 25, 2023
Merged

[Test Mv] quantization #51942

merged 9 commits into from
Mar 25, 2023

Conversation

Zheng-Bicheng
Copy link
Contributor

@Zheng-Bicheng Zheng-Bicheng commented Mar 21, 2023

PR types

Others

PR changes

Others

Describe

移动单测目录

@Zheng-Bicheng
Copy link
Contributor Author

修改介绍

  • 移动测试文件到 test/quantization目录
  • test/CMakeLists.txt包含quantization文件夹

@Zheng-Bicheng
Copy link
Contributor Author

关联Issues

@Zheng-Bicheng
Copy link
Contributor Author

@tianshuo78520a

@paddle-bot paddle-bot bot added the contributor External developers label Mar 21, 2023
@Zheng-Bicheng
Copy link
Contributor Author

Zheng-Bicheng commented Mar 22, 2023

关联PR

在PR中已经移动了tests目录下的单测文件,本次PR又移动了quantization目录。tests目录为空,直接删除。

@Zheng-Bicheng
Copy link
Contributor Author

@zhiqiu @tianshuo78520a 两位大佬好,我看到PR-CI-Kunlun-R200-Test这个CI出现了问题,我看了一下CI的报错,存在以下两个问题:

  • 有大量OP未添加到测试中
  • test_pad3d_op_xpu这个单测失败

这两个问题应该都和我的PR没关系?有办法解决这两个问题吗。

@tianshuo78520a
Copy link
Contributor

该任务处于测试中,属于非Required,可以不用关心

zhiqiu
zhiqiu previously approved these changes Mar 24, 2023
Copy link
Contributor

@zhiqiu zhiqiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@luotao1
Copy link
Contributor

luotao1 commented Mar 24, 2023

请解决下冲突,因为 test/CMakeLists.txt 频繁修改,你看下不要每次放到末尾,可以按字母序。不然每次合入一个你的PR,其他你的PR都冲突了。

@luotao1
Copy link
Contributor

luotao1 commented Mar 24, 2023

@tianshuo78520a 可以考虑在 test/CMakeLists.txt 下把一级目录都做出来,前面加#,这样避免大家频繁冲突
18479e7934a35f2918eaedd56ee7947c

@Zheng-Bicheng
Copy link
Contributor Author

@luotao1 这个放在末尾能解决问题吗?看上去好像还不能诶。冲突的原因应该是Git认为我修改的都是第12行,才导致冲突吧。因为修改的CMakeList.txt冲突的地方结构是包含在if语句中的,没啥好办法。

if(WITH_TESTING)
  add_subdirectory(cpp)
  add_subdirectory(legacy_test)
  add_subdirectory(book)
  add_subdirectory(quantization) -> 这里冲突了
endif()

@Zheng-Bicheng
Copy link
Contributor Author

@tianshuo78520a 可以考虑在 test/CMakeLists.txt 下把一级目录都做出来,前面加#,这样避免大家频繁冲突
18479e7934a35f2918eaedd56ee7947c

可以我提个PR哈

@Zheng-Bicheng
Copy link
Contributor Author

感谢涛姐,很好的解决办法🫡

add_subdirectory(book)
add_subdirectory(custom_kernel)
add_subdirectory(cpp)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

按字母序的话,cpp在custom_kernel前面,不过可以后面改

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#52131 在这个PR里提了,先整合后面一次性改掉?我想写个程序来排序,靠人工手动一个一个排有点头疼。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在这个PR里提了,先整合后面一次性改掉

可以的

@Zheng-Bicheng Zheng-Bicheng mentioned this pull request Mar 24, 2023
@luotao1 luotao1 merged commit 0c5a4ba into PaddlePaddle:develop Mar 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants