From 5e601a92ad678be694988c0603a38f753493b790 Mon Sep 17 00:00:00 2001 From: Zeng Jinle <32832641+sneaxiy@users.noreply.github.com> Date: Wed, 15 Jan 2020 01:20:27 -0600 Subject: [PATCH] polish grad op check (#22290) * polish grad op check, test=develop, test=document_fix * keep op_use_default_grad_maker.spec to avoid conflict, test=develop, test=document_fix --- .gitignore | 3 ++ paddle/scripts/paddle_build.sh | 15 +++--- tools/check_api_approvals.sh | 14 ++++-- tools/diff_use_default_grad_op_maker.py | 62 ++++++++++++------------- 4 files changed, 48 insertions(+), 46 deletions(-) diff --git a/.gitignore b/.gitignore index 8fa69fb9702..4a5c35bbc2e 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,9 @@ paddle/fluid/operators/distributed/send_recv.proto paddle/fluid/API.spec paddle/fluid/API_DEV.spec paddle/fluid/API_PR.spec +paddle/fluid/op_use_default_grad_maker_DEV.spec +paddle/fluid/op_use_default_grad_maker_PR.spec + *.DS_Store *.vs build/ diff --git a/paddle/scripts/paddle_build.sh b/paddle/scripts/paddle_build.sh index f63e580e617..c5146a63b95 100755 --- a/paddle/scripts/paddle_build.sh +++ b/paddle/scripts/paddle_build.sh @@ -465,16 +465,17 @@ function fetch_upstream_develop_if_not_exist() { if [ ! -e "$PADDLE_ROOT/.git/refs/remotes/upstream/$BRANCH" ]; then git fetch upstream # develop is not fetched fi -} +} function generate_upstream_develop_api_spec() { fetch_upstream_develop_if_not_exist - cur_branch=`git branch | grep \* | cut -d ' ' -f2` + cur_branch=`git branch | grep \* | cut -d ' ' -f2` git checkout -b develop_base_pr upstream/$BRANCH cmake_gen $1 build $2 - generate_api_spec "$1" "DEV" + git checkout $cur_branch + generate_api_spec "$1" "DEV" git branch -D develop_base_pr ENABLE_MAKE_CLEAN="ON" rm -rf ${PADDLE_ROOT}/build/Makefile ${PADDLE_ROOT}/build/CMakeCache.txt @@ -510,12 +511,10 @@ function generate_api_spec() { sed -i 's/arg0: str/arg0: unicode/g' $spec_path sed -i "s/\(.*Transpiler.*\).__init__ (ArgSpec(args=\['self'].*/\1.__init__ /g" $spec_path fi + + python ${PADDLE_ROOT}/tools/diff_use_default_grad_op_maker.py \ + ${PADDLE_ROOT}/paddle/fluid/op_use_default_grad_maker_${spec_kind}.spec - # TODO(paddle-dev): remove op_use_default_grad_op_maker.spec - if [ "$spec_kind" == "PR" ]; then - python ${PADDLE_ROOT}/tools/diff_use_default_grad_op_maker.py \ - ${PADDLE_ROOT}/paddle/fluid/op_use_default_grad_op_maker.spec - fi deactivate } diff --git a/tools/check_api_approvals.sh b/tools/check_api_approvals.sh index 15f97dc6510..92df022cf4d 100644 --- a/tools/check_api_approvals.sh +++ b/tools/check_api_approvals.sh @@ -5,7 +5,6 @@ fi PADDLE_ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}")/../" && pwd )" API_FILES=("CMakeLists.txt" - "paddle/fluid/op_use_default_grad_op_maker.spec" "paddle/fluid/framework/operator.h" "paddle/fluid/framework/tensor.h" "paddle/fluid/framework/details/op_registry.h" @@ -95,10 +94,7 @@ for API_FILE in ${API_FILES[*]}; do # NOTE: per_page=10000 should be ok for all cases, a PR review > 10000 is not human readable. # You can use http://caius.github.io/github_id/ to find Github user id. # 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,chenwhql 22561442,zhiqiu 6888866,seiriosPlus 5442383,gongweibao 10721757,saxon-zh 2870059,Boyan-Liu 31623103, zhouwei25 52485244, Aurelius84 9301846, liym27 33742067, zhhsplendid 7913861, kolinwei 22165420, liuwei1031 46661762, swtkiwi 27208573, juncaipeng 52520497, zhangting2020 26615455, JepsonWong 16509038. - 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" - check_approval 1 32832641 6836917 - elif [ "${API_FILE}" == "CMakeLists.txt" ];then + if [ "${API_FILE}" == "CMakeLists.txt" ];then echo_line="You must have one RD (luotao1 or XiaoguangHu01) approval for CMakeLists.txt, which manages the compilation parameter.\n" check_approval 1 6836917 46782768 elif [ "${API_FILE}" == "python/paddle/fluid/__init__.py" ];then @@ -254,6 +250,14 @@ if [ "${UNITTEST_FILE_CHANGED}" != "" ] && [ "${GIT_PR_ID}" != "" ]; then fi fi +DEV_OP_USE_DEFAULT_GRAD_MAKER_SPEC=${PADDLE_ROOT}/paddle/fluid/op_use_default_grad_maker_DEV.spec +PR_OP_USE_DEFAULT_GRAD_MAKER_SPEC=${PADDLE_ROOT}/paddle/fluid/op_use_default_grad_maker_PR.spec +ADDED_OP_USE_DEFAULT_GRAD_MAKER=`python ${PADDLE_ROOT}/tools/diff_use_default_grad_op_maker.py ${DEV_OP_USE_DEFAULT_GRAD_MAKER_SPEC} ${PR_OP_USE_DEFAULT_GRAD_MAKER_SPEC}` +if [ "${ADDED_OP_USE_DEFAULT_GRAD_MAKER}" != "" ]; then + echo_line="You must have one RD (sneaxiy (Recommend) or luotao1) approval because you use DefaultGradOpMaker for ${ADDED_OP_USE_DEFAULT_GRAD_MAKER}, which manages the grad_op memory optimization.\n" + check_approval 1 32832641 6836917 +fi + if [ -n "${echo_list}" ];then echo "****************" echo -e "${echo_list[@]}" diff --git a/tools/diff_use_default_grad_op_maker.py b/tools/diff_use_default_grad_op_maker.py index 9e362f611bb..bab07123d53 100644 --- a/tools/diff_use_default_grad_op_maker.py +++ b/tools/diff_use_default_grad_op_maker.py @@ -20,47 +20,43 @@ import paddle.fluid as fluid import sys -def get_op_diff(filename): - ops_created_by_py_func = set( - fluid.core._get_use_default_grad_op_desc_maker_ops()) +def generate_spec(filename): + with open(filename, 'w') as f: + ops = fluid.core._get_use_default_grad_op_desc_maker_ops() + for op in ops: + f.write(op + '\n') - with open(filename, 'r') as f: - ops_read_from_file = set([line.strip() for line in f.readlines()]) - - diff_ops = [] - for op in ops_read_from_file: - if op not in ops_created_by_py_func: - diff_ops.append(op) - else: - ops_created_by_py_func.remove(op) +def read_spec(filename): + with open(filename, 'r') as f: + return set([line.strip() for line in f.readlines()]) - err_msg = [] - diff_ops = list(diff_ops) - if len(diff_ops) > 0: - err_msg.append('Added grad op with DefaultGradOpDescMaker: ' + str( - diff_ops)) - ops_created_by_py_func = list(ops_created_by_py_func) - if len(ops_created_by_py_func) > 0: - err_msg.append('Remove grad op with DefaultGradOpDescMaker: ' + str( - ops_created_by_py_func)) +def get_spec_diff(dev_filename, pr_filename): + ops_dev = read_spec(dev_filename) + ops_pr = read_spec(pr_filename) - return err_msg + added_ops = [] + removed_ops = [] + for op in ops_pr: + if op not in ops_dev: + added_ops.append(op) + else: + removed_ops.append(op) -if len(sys.argv) != 2: - print('Usage: python diff_use_default_grad_op_maker.py [filepath]') - sys.exit(1) + return added_ops -file_path = str(sys.argv[1]) -err_msg = get_op_diff(file_path) -if len(err_msg) > 0: - _, filename = os.path.split(file_path) - print('File `{}` is wrong compared to your PR revision!'.format(filename)) +if len(sys.argv) == 2: + generate_spec(sys.argv[1]) +elif len(sys.argv) == 3: + added_ops = get_spec_diff(sys.argv[1], sys.argv[2]) + if added_ops: + print(added_ops) +else: print( - 'Please use `python generate_op_use_grad_op_desc_maker_spec.py [filepath]` to generate new `{}` file'. - format(filename)) - print('Error message is: ' + '; '.join(err_msg)) + 'Usage 1: python diff_use_default_grad_op_maker.py [filepath] to generate new spec file\n' + 'Usage 2: python diff_use_default_grad_op_maker.py [dev_filepath] [pr_filepath] to diff spec file' + ) sys.exit(1) -- GitLab