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

Refine the generation process of swig's python wrapper #1099

Closed
wants to merge 13 commits into from

Conversation

gangliao
Copy link
Contributor

@gangliao gangliao commented Jan 9, 2017

Fix #1083

cmake/util.cmake Outdated
@@ -95,32 +95,12 @@ function(link_paddle_exe TARGET_NAME)
paddle_parameter
paddle_proto
paddle_cuda
${METRIC_LIBS}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

重复

${RDMA_LD_FLAGS}
${RDMA_LIBS}
${START_END}
)

Copy link
Collaborator

Choose a reason for hiding this comment

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

# TODO(yuyang18) : make wheel name calculated by cmake

这个事情其实还是没有fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

revert 了

)

generate_python_api(python_swig_sources)
IF(APPLE)
SET(ARCHIVE_START "-undefined dynamic_lookup -Wl,-all_load")
Copy link
Collaborator

Choose a reason for hiding this comment

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

记得 -all_load 不太等于ARCHIVE_START...

add_custom_command(OUTPUT ${PROJ_ROOT}/paddle/dist/.timestamp
COMMAND mv ${CMAKE_CURRENT_BINARY_DIR}/swig_paddle.py ${PROJ_ROOT}/paddle/py_paddle
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个可以重新搞一下。

1、应该用cp而不是用 mv,否则,${CMAKE_CURRENT_BINARY_DIR}/swig_paddle.py 会永远不存在,然后每次make的时候都会编译。

2、之前用timestamp来标记这个OUTPUT其实是个trick

  • 理论上应该output的是那个wheel包的名字,但是python的wheel包的名字是运行时算出来的,就没在cmake里面搞。
  • 现在既然有复制的过程。可以直接用_swig_paddle.so作为输出之类的。

else()
set(GFLAGS_LOCATION ${GFLAGS_LIBRARIES})
endif()
FILE(GLOB PY_PADDLE_PYTHON_FILES ${PROJ_ROOT}/paddle/py_paddle/*.py)
Copy link
Collaborator

Choose a reason for hiding this comment

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

GLOB => GLOB_RECURSE

因为我们可能会在这个包里面加一些其他的子目录,子目录下面也有PY文件。

Copy link
Contributor Author

@gangliao gangliao Jan 12, 2017

Choose a reason for hiding this comment

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

删掉了 好像这里没起什么作用 以后新增的py,直接在https://github.com/PaddlePaddle/Paddle/pull/1099/files#diff-409abc2eda4ed1dba2c69defc807748dR22 里面加,直观一些。

@gangliao
Copy link
Contributor Author

@reyoung done

@jacquesqiao jacquesqiao self-requested a review March 21, 2017 14:55
@gangliao gangliao closed this Mar 22, 2017
wangxicoding pushed a commit to wangxicoding/Paddle that referenced this pull request Dec 9, 2021
* modify transforner-rst

* modify electra tokenizer

* modify electramodel

* modify electra criterion

* modify models

* modify tokenizers

* fix errors

* update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants