add timeout test #261
add timeout test #261
Conversation
gongweibao
requested review from
Yancey1989,
wangkuiyi,
helinwang and
typhoonzero
| @@ -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 | |||
| timeout -s SIGKILL 30 bash -c \ | |||
There was a problem hiding this comment.
这里用了timeout，是否要在开头加上set -e呢，这样只要timeout之后脚本会推出而不是继续执行？
There was a problem hiding this comment.
根据这里的讨论，最好不用set -e
| timeout -s SIGKILL 30 bash -c "jupyter nbconvert --to python $(basename $file) --stdout | python" | ||
| fi | ||
|
|
||
| if [[ $? -ne 0 && $? -ne 124 && $? -ne 137 ]]; then |
There was a problem hiding this comment.
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还是需要的
| @@ -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 | |||
There was a problem hiding this comment.
为什么recommender system的处理方式和其他chapter不同呢？
看上去是因为不想调用绘制函数？ @jacquesqiao 和其他人正在给所有chapters加上绘图功能，是不是到时候都需要特殊处理呢？
There was a problem hiding this comment.
是有这个问题。调用绘制函数会有两个问题
- 转换的过程中会加入get_ipython()的调用，这个直接运行会失败
- matplot lib绘制图像也会要求x display
不过，如果都加上了，反而不用特殊处理了。因为大家都一样用sed替换了。没加上的反而特殊处理了。
There was a problem hiding this comment.
可否可 @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, ...)
There was a problem hiding this comment.
1，测试了下，只要调用 import matplotlib.pyplot 就会挂
2，我看看能不能通过其他方式选择是否调用 import matplotlib.pyplot
There was a problem hiding this comment.
我记得import 可以写在函数定义里的？
def new(dry_run=false, ...):
if not dry_run:
import paddle.v2.plot.costThere was a problem hiding this comment.
PaddlePaddle/Paddle#1712
测试了下，通过一个环境变量来决定是否import matplotlib.pyplot。
测试的时候只需要
export DISABLE_PLOT=True就不会挂了
There was a problem hiding this comment.
龙飞的提交了以后告诉我一声，我根据你的pr进行一下修改，然后再merge进去 @jacquesqiao
|
哦。下次提PR的时候我需要把思路和碰到、解决的问题写出来。。。这样更有利于大家看 |
|
|
2.增加时间的限制以便可以对所有的ipynb做初步的正确性的检查。当然，这种方式不能检查收敛性的正确性。收敛需要的时间太长。