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

@@ -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 

python/paddle/fluid/tests/unittests/test_pybind_inference.py → 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__':

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

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")