From be6a639655772955654a0ab0d2b36bcee6800aa2 Mon Sep 17 00:00:00 2001 From: liym27 <33742067+liym27@users.noreply.github.com> Date: Mon, 9 Dec 2019 14:49:48 +0800 Subject: [PATCH] Add CI for checking Input/Output/Attr of modified Ops (#21522) * add shell scripts. test=develop * rename test_pybind_inference to test_pybind_interface and print repeat process in check_op_desc.py. test=develop * add approval RD. test=develop --- paddle/scripts/paddle_build.sh | 7 ++++++- ...ybind_inference.py => test_pybind_interface.py} | 14 +++++++++++++- tools/check_api_approvals.sh | 11 +++++++++-- tools/check_op_desc.py | 6 +++--- 4 files changed, 31 insertions(+), 7 deletions(-) rename python/paddle/fluid/tests/unittests/{test_pybind_inference.py => test_pybind_interface.py} (70%) diff --git a/paddle/scripts/paddle_build.sh b/paddle/scripts/paddle_build.sh index 1315771bdf5..5bf3e4d5b0b 100755 --- a/paddle/scripts/paddle_build.sh +++ b/paddle/scripts/paddle_build.sh @@ -484,12 +484,17 @@ function generate_api_spec() { virtualenv .${spec_kind}_env source .${spec_kind}_env/bin/activate pip install ${PADDLE_ROOT}/build/python/dist/*whl - spec_path=${PADDLE_ROOT}/paddle/fluid/API_${spec_kind}.spec python ${PADDLE_ROOT}/tools/print_signatures.py paddle.fluid > $spec_path + # used to log op_register data_type op_type_path=${PADDLE_ROOT}/paddle/fluid/OP_TYPE_${spec_kind}.spec python ${PADDLE_ROOT}/tools/check_op_register_type.py > $op_type_path + + # print all ops desc in dict to op_desc_path + op_desc_path=${PADDLE_ROOT}/paddle/fluid/OP_DESC_${spec_kind}.spec + python ${PADDLE_ROOT}/tools/print_op_desc.py > $op_desc_path + awk -F '(' '{print $NF}' $spec_path >${spec_path}.doc awk -F '(' '{$NF="";print $0}' $spec_path >${spec_path}.api if [ "$1" == "cp35-cp35m" ] || [ "$1" == "cp36-cp36m" ] || [ "$1" == "cp37-cp37m" ]; then diff --git a/python/paddle/fluid/tests/unittests/test_pybind_inference.py b/python/paddle/fluid/tests/unittests/test_pybind_interface.py similarity index 70% rename from python/paddle/fluid/tests/unittests/test_pybind_inference.py rename to python/paddle/fluid/tests/unittests/test_pybind_interface.py index ffe05f531a6..164bc564bdf 100644 --- a/python/paddle/fluid/tests/unittests/test_pybind_inference.py +++ b/python/paddle/fluid/tests/unittests/test_pybind_interface.py @@ -22,7 +22,19 @@ class TestPybindInference(unittest.TestCase): # call get_op_attrs_default_value for c++ coverage rate def test_get_op_attrs_default_value(self): - core.get_op_attrs_default_value(cpt.to_bytes("fc")) + core.get_op_attrs_default_value(cpt.to_bytes("fill_constant")) + + # the default values of Op 'fill_constant' + # + # {"str_value": "", + # "force_cpu": false, + # "value": 1.0, + # "op_role_var": [], + # "shape": [], + # "op_namescope": "", + # "test_attr_1": 1.0, + # "op_callstack": [], + # "op_role": 4096} if __name__ == '__main__': diff --git a/tools/check_api_approvals.sh b/tools/check_api_approvals.sh index e7fdca27a8d..3c70dbb6c82 100644 --- a/tools/check_api_approvals.sh +++ b/tools/check_api_approvals.sh @@ -73,11 +73,17 @@ if [ "$op_type_spec_diff" != "" ]; then check_approval 1 9301846 33742067 7913861 fi +op_desc_diff=`python ${PADDLE_ROOT}/tools/check_op_desc.py ${PADDLE_ROOT}/paddle/fluid/OP_DESC_DEV.spec ${PADDLE_ROOT}/paddle/fluid/OP_DESC_PR.spec` +if [ "$op_desc_diff" != "" ]; then + echo_line="You must have one RD (liym27 (Recommend), zhhsplendid, Aurelius84, lanxianghit or phlrain) approval for the changes of Inputs/Output/Attrs of OPs. The changes of OPs will cause that the new version inference fails to load model trained by the old version.\n" + check_approval 1 33742067 7913861 9301846 47554610 43953930 +fi + for API_FILE in ${API_FILES[*]}; do API_CHANGE=`git diff --name-only upstream/$BRANCH | grep "${API_FILE}" | grep -v "/CMakeLists.txt" || true` echo "checking ${API_FILE} change, PR: ${GIT_PR_ID}, changes: ${API_CHANGE}" if [ "${API_CHANGE}" ] && [ "${GIT_PR_ID}" != "" ]; then - # NOTE: per_page=10000 should be ok for all cases, a PR review > 10000 is not human readable.q + # NOTE: per_page=10000 should be ok for all cases, a PR review > 10000 is not human readable. # approval_user_list: XiaoguangHu01 46782768,Xreki 12538138,luotao1 6836917,sneaxiy 32832641,qingqing01 7845005,guoshengCS 14105589,heavengate 12605721,kuke 3064195,Superjomn 328693,lanxianghit 47554610,cyj1986 39645414,hutuxian 11195205,frankwhzhang 20274488,nepeplwu 45024560,Dianhai 38231817,JiabinYang 22361972,chenwhql 22561442,zhiqiu 6888866,seiriosPlus 5442383,gongweibao 10721757,saxon-zh 2870059,Boyan-Liu 2870059, zhouwei25 52485244, Aurelius84 9301846, liym27 33742067, zhhsplendid 7913861. if [ "${API_FILE}" == "paddle/fluid/op_use_default_grad_op_maker.spec" ];then echo_line="You must have one RD (sneaxiy (Recommend) or luotao1) approval for op_use_default_grad_op_maker.spec, which manages the grad_op memory optimization.\n" @@ -113,7 +119,7 @@ fi HAS_DEFINE_FLAG=`git diff -U0 upstream/$BRANCH |grep -o -m 1 "DEFINE_int32" |grep -o -m 1 "DEFINE_bool" | grep -o -m 1 "DEFINE_string" || true` if [ ${HAS_DEFINE_FLAG} ] && [ "${GIT_PR_ID}" != "" ]; then echo_line="You must have one RD lanxianghit approval for the usage (either add or delete) of DEFINE_int32/DEFINE_bool/DEFINE_string flag.\n" - check_approval 1 47554610 + check_approval 1 47554610 fi HAS_UNITTEST_SKIP=`git diff -U0 upstream/$BRANCH | grep "^+[[:space:]]\{0,\}@unittest.skip" || true` @@ -167,6 +173,7 @@ fi python ${PADDLE_ROOT}/tools/diff_api.py ${PADDLE_ROOT}/paddle/fluid/API_DEV.spec ${PADDLE_ROOT}/paddle/fluid/API_PR.spec python ${PADDLE_ROOT}/tools/check_op_register_type.py ${PADDLE_ROOT}/paddle/fluid/OP_TYPE_DEV.spec ${PADDLE_ROOT}/paddle/fluid/OP_TYPE_PR.spec +python ${PADDLE_ROOT}/tools/check_op_desc.py ${PADDLE_ROOT}/paddle/fluid/OP_DESC_DEV.spec ${PADDLE_ROOT}/paddle/fluid/OP_DESC_PR.spec if [ -n "${echo_list}" ]; then exit 1 diff --git a/tools/check_op_desc.py b/tools/check_op_desc.py index 92324ea0070..ae3e3cafdaf 100644 --- a/tools/check_op_desc.py +++ b/tools/check_op_desc.py @@ -164,9 +164,8 @@ def compare_op_desc(origin_op_desc, new_op_desc): def print_error_message(error_message): - print("Op desc error is:") + print("Op desc error for the changes of Inputs/Outputs/Attrs of OPs:\n") for op_name in error_message: - print("-" * 30) print("For OP '{}':".format(op_name)) # 1. print inputs error message @@ -217,7 +216,6 @@ def print_error_message(error_message): print( " * The arg '{}' of attr '{}' is changed: from '{}' to '{}'.". format(arg, name, ori_value, new_value)) - print("-" * 30) def print_repeat_process(): @@ -245,7 +243,9 @@ if len(sys.argv) == 3: error_message = compare_op_desc(origin_op_desc, new_op_desc) if error: + print("-" * 30) print_error_message(error_message) print_repeat_process() + print("-" * 30) else: print("Usage: python check_op_desc.py OP_DESC_DEV.spec OP_DESC_PR.spec") -- GitLab