Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

add timeout test #261

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

add timeout test #261

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a49e7a
add timeout
gongweibao Mar 28, 2017
f0f5b27
fix
gongweibao Mar 28, 2017
6345129
fix
gongweibao Mar 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions .tools/convert-markdown-into-ipynb-and-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/bin/bash
command -v go >/dev/null 2>&1
if [ $? -ne 0 ]; then
if [[ $? -ne 0 ]]; then
echo >&2 "Please install go https://golang.org/doc/install#install"
exit 1
fi
Expand All @@ -13,7 +13,7 @@ cd $cur_path/../
#convert md to ipynb
for file in */{README,README\.en}.md ; do
~/go/bin/markdown-to-ipynb < $file > ${file%.*}".ipynb"
if [ $? -ne 0 ]; then
if [[ $? -ne 0 ]]; then
echo >&2 "markdown-to-ipynb $file error"
exit 1
fi
Expand All @@ -29,7 +29,19 @@ for file in */{README,README\.en}.ipynb ; do
cd $(dirname $file) > /dev/null

echo "begin test $file"
jupyter nbconvert --to python $(basename $file) --stdout | python
if [[ $(dirname $file) == "08.recommender_system" ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

为什么recommender system的处理方式和其他chapter不同呢？

看上去是因为不想调用绘制函数？ @jacquesqiao 和其他人正在给所有chapters加上绘图功能，是不是到时候都需要特殊处理呢？

Copy link
Contributor Author

@gongweibao gongweibao Mar 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

是有这个问题。调用绘制函数会有两个问题

  • 转换的过程中会加入get_ipython()的调用，这个直接运行会失败
  • matplot lib绘制图像也会要求x display

不过，如果都加上了，反而不用特殊处理了。因为大家都一样用sed替换了。没加上的反而特殊处理了。

Copy link
Collaborator

@wangkuiyi wangkuiyi Mar 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

可否可 @jacquesqiao 聊一下这个问题？我理解绘图会封装到 @jacquesqiao 写的 paddle.v2.plot.X 里。是不是可以调用方式是：

import paddle.v2.plot.cost

c = plot.cost.new(....)
c.add_point(x, y)
c.draw()

然后通过给new函数增加一个参数 dry_run=true，让new函数以及之后和绘图相关的功能都被disable掉？

也就是用sed把

c = plot.cost.new(...)

变成

c = plot.cost.new(dry_run=true, ...)

Copy link
Member

@jacquesqiao jacquesqiao Mar 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

1，测试了下，只要调用 import matplotlib.pyplot 就会挂
2，我看看能不能通过其他方式选择是否调用 import matplotlib.pyplot

Copy link
Collaborator

Choose a reason for hiding this comment

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

我记得import 可以写在函数定义里的？

def new(dry_run=false, ...):
  if not dry_run:
    import paddle.v2.plot.cost

Copy link
Member

Choose a reason for hiding this comment

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

PaddlePaddle/Paddle#1712
测试了下，通过一个环境变量来决定是否import matplotlib.pyplot。
测试的时候只需要

export DISABLE_PLOT=True

就不会挂了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

龙飞的提交了以后告诉我一声，我根据你的pr进行一下修改，然后再merge进去 @jacquesqiao

Copy link
Member

Choose a reason for hiding this comment

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

好的

timeout -s SIGKILL 30 bash -c \
Copy link
Contributor

Choose a reason for hiding this comment

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

这里用了timeout，是否要在开头加上set -e呢，这样只要timeout之后脚本会推出而不是继续执行？

Copy link
Contributor Author

@gongweibao gongweibao Mar 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

根据这里的讨论，最好不用set -e

"jupyter nbconvert --to python $(basename $file) --stdout | \
sed 's/get_ipython()\.magic(.*'\''matplotlib inline'\'')/\#matplotlib inline/g' | \
sed '/^# coding: utf-8/a\import matplotlib\nmatplotlib.use('\''Agg'\'')' | python"
else
timeout -s SIGKILL 30 bash -c "jupyter nbconvert --to python $(basename $file) --stdout | python"
fi

if [[ $? -ne 0 && $? -ne 124 && $? -ne 137 ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

为什么124和137返回是正确的呢？

Copy link
Contributor Author

@gongweibao gongweibao Mar 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

If the command times out, and --preserve-status is not set, then exit
with status 124. Otherwise, exit with the status of COMMAND. If no
signal is specified, send the TERM signal upon timeout. The TERM
signal kills any process that does not block or catch that signal.
It may be necessary to use the KILL (9) signal, since this signal
cannot be caught, in which case the exit status is 128+9 rather than
124.

确实不用写124，137还是需要的

echo >&2 "exec $file error!"
exit 1
fi

popd > /dev/null
#break
Expand Down