From 1ab4101d6cc11e27c5ba35f2d5a3f50983435e91 Mon Sep 17 00:00:00 2001 From: Leo Chen Date: Tue, 21 Jul 2020 21:09:29 +0800 Subject: [PATCH] add ci check for changing op-related api without core.ops, test=develop (#25596) * add ci check for changing op-related api without core.ops, test=develop * generate api_source_md5 file when build, test=develop * add failed example, test=develop * add failed example, test=develop * handle exception, test=develop --- paddle/scripts/paddle_build.sh | 4 + python/paddle/fluid/layers/nn.py | 2 +- tools/check_api_approvals.sh | 10 +- tools/check_api_source_without_core_ops.py | 49 +++++++++ ...t_ops.py => count_api_without_core_ops.py} | 101 +++++++++++++----- 5 files changed, 137 insertions(+), 29 deletions(-) create mode 100644 tools/check_api_source_without_core_ops.py rename tools/{count_api_without_ops.py => count_api_without_core_ops.py} (53%) diff --git a/paddle/scripts/paddle_build.sh b/paddle/scripts/paddle_build.sh index a2e590ac556..0b6b006bbb2 100755 --- a/paddle/scripts/paddle_build.sh +++ b/paddle/scripts/paddle_build.sh @@ -584,6 +584,10 @@ function generate_api_spec() { op_desc_path=${PADDLE_ROOT}/paddle/fluid/OP_DESC_${spec_kind}.spec python ${PADDLE_ROOT}/tools/print_op_desc.py > $op_desc_path + # print api and the md5 of source code of the api. + api_source_md5_path=${PADDLE_ROOT}/paddle/fluid/API_${spec_kind}.source.md5 + python ${PADDLE_ROOT}/tools/count_api_without_core_ops.py -p paddle > $api_source_md5_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/layers/nn.py b/python/paddle/fluid/layers/nn.py index 083c2ffbe36..29648fcf157 100644 --- a/python/paddle/fluid/layers/nn.py +++ b/python/paddle/fluid/layers/nn.py @@ -6207,7 +6207,7 @@ def squeeze(input, axes, name=None): check_variable_and_dtype( input, 'input', ['float16', 'float32', 'float64', 'int8', 'int32', 'int64'], 'squeeze') - check_type(axes, 'axes', list, 'squeeze') + check_type(axes, 'axes', (list, tuple), 'squeeze') out = helper.create_variable_for_type_inference(dtype=input.dtype) x_shape = helper.create_variable_for_type_inference(dtype=input.dtype) helper.append_op( diff --git a/tools/check_api_approvals.sh b/tools/check_api_approvals.sh index ef8f9e455f5..d1d7e64299e 100644 --- a/tools/check_api_approvals.sh +++ b/tools/check_api_approvals.sh @@ -80,10 +80,18 @@ if [ "$api_doc_spec_diff" != "" ]; then check_approval 1 2870059 29231 27208573 28379894 11935832 fi +api_spec_diff=`python ${PADDLE_ROOT}/tools/check_api_source_without_core_ops.py ${PADDLE_ROOT}/paddle/fluid/API_DEV.source.md5 ${PADDLE_ROOT}/paddle/fluid/API_PR.source.md5` +if [ "$api_spec_diff" != "" ]; then + echo_line="You must have one RD (zhiqiu (Recommend) or phlrain) approval for the api change for the opreator-related api without 'core.ops'.\n" + echo_line="${echo_line}For more details, please click [https://github.com/PaddlePaddle/Paddle/wiki/paddle_api_development_manual.md]\n" + echo_line="${echo_line}Related APIs: ${api_spec_diff}" + check_approval 1 6888866 43953930 +fi + op_type_spec_diff=`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` if [ "$op_type_spec_diff" != "" ]; then echo_line="You must have one RD (Aurelius84 (Recommend) or liym27 or zhhsplendid)approval for the data_type registration of new operator. More data_type of new operator should be registered in your PR. Please make sure that both float/double (or int/int64_t) have been registered.\n For more details, please click [https://github.com/PaddlePaddle/Paddle/wiki/Data-types-of-generic-Op-must-be-fully-registered].\n" - check_approval 1 9301846 33742067 7913861 + check_approval 1 9j301846 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` diff --git a/tools/check_api_source_without_core_ops.py b/tools/check_api_source_without_core_ops.py new file mode 100644 index 00000000000..ef9b5851185 --- /dev/null +++ b/tools/check_api_source_without_core_ops.py @@ -0,0 +1,49 @@ +# Copyright (c) 2020 PaddlePaddle Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import print_function +import difflib +import sys +import importlib +import os +import count_api_without_core_ops + +with open(sys.argv[1], 'r') as f: + origin = f.read() + origin = origin.splitlines() + +with open(sys.argv[2], 'r') as f: + new = f.read() + new = new.splitlines() + +differ = difflib.Differ() +result = differ.compare(origin, new) + +api_with_ops, api_without_ops = count_api_without_core_ops.get_apis_with_and_without_core_ops( + ['paddle']) + +error = False +# get all diff apis +# check if the changed api's source code contains append_op but not core.ops +diffs = [] +for each_diff in result: + if each_diff[0] == '+': + api_name = each_diff.split(' ')[1].strip() + if api_name in api_without_ops: + error = True + diffs += [api_name] + +if error: + for each_diff in diffs: + print(each_diff) diff --git a/tools/count_api_without_ops.py b/tools/count_api_without_core_ops.py similarity index 53% rename from tools/count_api_without_ops.py rename to tools/count_api_without_core_ops.py index 84dd9a6b2f6..99e84074158 100644 --- a/tools/count_api_without_ops.py +++ b/tools/count_api_without_core_ops.py @@ -11,12 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -""" -List all operator-raleated APIs that contains append_op but not core.ops.xx. -Usage: - python ./count_api_without_ops.py paddle -""" from __future__ import print_function import importlib @@ -28,7 +23,7 @@ import hashlib import six import functools -visited_modules = set() +__all__ = ['get_apis_with_and_without_core_ops', ] # APIs that should not be printed into API.spec omitted_list = [ @@ -37,11 +32,14 @@ omitted_list = [ "paddle.fluid.io.ComposeNotAligned.__init__", ] -api_with_ops = [] -api_without_ops = [] +def md5(doc): + hash = hashlib.md5() + hash.update(str(doc).encode('utf-8')) + return hash.hexdigest() -def queue_dict(member, cur_name): + +def split_with_and_without_core_ops(member, cur_name): if cur_name in omitted_list: return @@ -55,23 +53,40 @@ def queue_dict(member, cur_name): api_with_ops.append(cur_name) else: api_without_ops.append(cur_name) + except: + # If getsource failed (pybind API or function inherit from father class), just skip + pass + - except Exception as e: # special for PyBind method +def get_md5_of_func(member, cur_name): + if cur_name in omitted_list: + return + + doc_md5 = md5(member.__doc__) + + if inspect.isclass(member): + pass + else: + try: + source = inspect.getsource(member) + func_dict[cur_name] = md5(source) + except: + # If getsource failed (pybind API or function inherit from father class), just skip pass -def visit_member(parent_name, member): +def visit_member(parent_name, member, func): cur_name = ".".join([parent_name, member.__name__]) if inspect.isclass(member): - queue_dict(member, cur_name) + func(member, cur_name) for name, value in inspect.getmembers(member): if hasattr(value, '__name__') and (not name.startswith("_") or name == "__init__"): - visit_member(cur_name, value) + visit_member(cur_name, value, func) elif inspect.ismethoddescriptor(member): return elif callable(member): - queue_dict(member, cur_name) + func(member, cur_name) elif inspect.isgetsetdescriptor(member): return else: @@ -94,7 +109,7 @@ def is_primitive(instance): return False -def visit_all_module(mod): +def visit_all_module(mod, visited, func): mod_name = mod.__name__ if mod_name != 'paddle' and not mod_name.startswith('paddle.'): return @@ -102,10 +117,10 @@ def visit_all_module(mod): if mod_name.startswith('paddle.fluid.core'): return - if mod in visited_modules: + if mod in visited: return - visited_modules.add(mod) + visited.add(mod) for member_name in ( name @@ -122,19 +137,51 @@ def visit_all_module(mod): continue if inspect.ismodule(instance): - visit_all_module(instance) + visit_all_module(instance, visited, func) else: - visit_member(mod.__name__, instance) + visit_member(mod.__name__, instance, func) + +def get_apis_with_and_without_core_ops(modules): + global api_with_ops, api_without_ops + api_with_ops = [] + api_without_ops = [] + for m in modules: + visit_all_module( + importlib.import_module(m), set(), split_with_and_without_core_ops) + return api_with_ops, api_without_ops -modules = sys.argv[1].split(",") -for m in modules: - visit_all_module(importlib.import_module(m)) -print('api_with_ops:', len(api_with_ops)) -print('\n'.join(api_with_ops)) +def get_api_source_desc(modules): + global func_dict + func_dict = collections.OrderedDict() + for m in modules: + visit_all_module(importlib.import_module(m), set(), get_md5_of_func) + return func_dict -print('\n==============\n') -print('api_without_ops:', len(api_without_ops)) -print('\n'.join(api_without_ops)) +if __name__ == "__main__": + if len(sys.argv) > 1: + modules = sys.argv[2].split(",") + if sys.argv[1] == '-c': + api_with_ops, api_without_ops = get_apis_with_and_without_core_ops( + modules) + + print('api_with_ops:', len(api_with_ops)) + print('\n'.join(api_with_ops)) + print('\n==============\n') + print('api_without_ops:', len(api_without_ops)) + print('\n'.join(api_without_ops)) + + if sys.argv[1] == '-p': + func_dict = get_api_source_desc(modules) + for name in func_dict: + print(name, func_dict[name]) + + else: + print("""Usage: + 1. Count and list all operator-raleated APIs that contains append_op but not core.ops.xx. + python ./count_api_without_core_ops.py -c paddle + 2. Print api and the md5 of source code of the api. + python ./count_api_without_core_ops.py -p paddle + """) -- GitLab