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

Update LSTM examples #5493

Merged
merged 21 commits into from
Dec 30, 2022
Merged

Update LSTM examples #5493

merged 21 commits into from
Dec 30, 2022

Conversation

edencfc
Copy link
Contributor

@edencfc edencfc commented Dec 7, 2022

Update LSTM forecast examples

@paddle-bot
Copy link

paddle-bot bot commented Dec 7, 2022

感谢你贡献飞桨文档,文档预览构建中,Docs-New 跑完后即可预览,预览链接:http://preview-pr-5493.paddle-docs-preview.paddlepaddle.org.cn/documentation/docs/zh/api/index_cn.html
预览工具的更多说明,请参考:[Beta]飞桨文档预览工具

Copy link
Collaborator

@wj-Mcat wj-Mcat left a comment

Choose a reason for hiding this comment

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

你的案例非常好,不过我这边针对于 auto-encoder有几点建议:

  1. 添加 requirements.txt文件,从而提供你这个案例中所需要的所有依赖包。
  2. 移除 temp_forecast.ipynb 文件
  3. !wget -O NAB.zip https://bj.bcebos.com/* 这个链接无法下载对应的文件,请确保无误之后再次上传。
  4. 参数命名上面的小建议:data_reader -> data_loader, 模型权重保存是:’model’ -> “best_model.pdparams”
  5. 第六小节中,可以使用 subfig 将图弄到一起,避免 notebook 的篇幅过长。

@wj-Mcat
Copy link
Collaborator

wj-Mcat commented Dec 14, 2022

你的案例非常好,不过我这边针对stock_forecast有几点建议:

  1. 添加 requirements.txt文件,从而提供你这个案例中所需要的所有依赖包。
  2. 你的示例如果只能够运行在 2.4.* 上面,我建议你在 notebook 当中用 assert写上一个版本判断逻辑,这样可强制开发者升级版本。
  3. 我本地运行import dnutils的代码报错:
File ~/miniconda3/envs/paddlenlp/lib/python3.8/site-packages/dnutils/__init__.py:8, in <module>
      3 from .debug import (out, stop, trace, stoptrace, err, warn, ok)
      5 from .tools import (ifnone, ifnot, allnone, allin, allequal, allnot, edict, idxif, first, fst, last, LinearScale,
      6                     mapstr, chunks, pairwise, project, isnone, is_not_none, where, where_not, str2bool)
----> 8 from .signals import add_handler, rm_handler, enable_ctrlc
     10 from .threads import Lock, RLock, Condition, Event, Semaphore, BoundedSemaphore, Barrier, Relay, Thread, \
     11     SuspendableThread, sleep, waitabout, Timer
     13 from .logs import (loggers, newlogger, getlogger, DEBUG, INFO, WARNING, ERROR, CRITICAL, expose, inspect,
     14                    active_exposures, exposure, set_exposure_dir)

File ~/miniconda3/envs/paddlenlp/lib/python3.8/site-packages/dnutils/signals.py:25, in <module>
     23 SIGBUS = signal_.SIGBUS
     24 SIGCHLD = signal_.SIGCHLD
---> 25 SIGCLD = signal_.SIGCLD
     26 SIGCONT = signal_.SIGCONT
     27 SIGFPE = signal_.SIGFPE

AttributeError: module 'signal' has no attribute 'SIGCLD'

其版本也是最新的:dnutils==0.3.12

我建议去掉dnutils的依赖,这个包star 为0,不推荐使用。如果想要使用,请多环境下测试,确保程序能够正常运行。

@wj-Mcat
Copy link
Collaborator

wj-Mcat commented Dec 16, 2022

请在这个 pr 上面更新,你本地push 的时候记得要push origin lstm-01 branch。

我看你已经发了好几个 pr,后续我将把其他的几个 pr 都关掉,所有的commit 将会集中在这个 pr 中来。

@edencfc
Copy link
Contributor Author

edencfc commented Dec 16, 2022

auto-encoder是文档已有的示例,我提交的是气温趋势预测和股票趋势预测两个示例,也就是temp_forecast.ipynb 和 stock_forecast.ipynb

@edencfc
Copy link
Contributor Author

edencfc commented Dec 16, 2022

三个示例不是一起的,不建议文档添加requirements.txt文件,看了下其它方向的示例,也没有都没有加requirements.txt文件

@wj-Mcat
Copy link
Collaborator

wj-Mcat commented Dec 16, 2022

三个示例不是一起的,不建议文档添加requirements.txt文件,看了下其它方向的示例,也没有都没有加requirements.txt文件

我看你的依赖是有点多,所以用 requirements.txt 是更规范一点,如果你觉得麻烦,就需要在notebook当中都写上。

@wj-Mcat
Copy link
Collaborator

wj-Mcat commented Dec 16, 2022

不好意思我把你其他两个pr 给关掉了,如果内容不是重复的话,你可以 reopen。

等你调整完了,你可以艾特我来继续 review 。

@edencfc
Copy link
Contributor Author

edencfc commented Dec 17, 2022

三个示例不是一起的,不建议文档添加requirements.txt文件,看了下其它方向的示例,也没有都没有加requirements.txt文件

我看你的依赖是有点多,所以用 requirements.txt 是更规范一点,如果你觉得麻烦,就需要在notebook当中都写上。

现在每个notebook的依赖最多2个,都写上了(常用的numpy、pandas、matplotlib、sklearn不用写吧?)

@edencfc
Copy link
Contributor Author

edencfc commented Dec 17, 2022

不好意思我把你其他两个pr 给关掉了,如果内容不是重复的话,你可以 reopen。

等你调整完了,你可以艾特我来继续 review 。

已经在这个pr里更新了,可以看到么?

@momozi1996
Copy link
Collaborator

不好意思我把你其他两个pr 给关掉了,如果内容不是重复的话,你可以 reopen。
等你调整完了,你可以艾特我来继续 review 。

已经在这个pr里更新了,可以看到么?

@wj-Mcat hi~ @edencfc 大家对齐一下更改的状态?

Copy link
Collaborator

@wj-Mcat wj-Mcat left a comment

Choose a reason for hiding this comment

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

LGTM

@wj-Mcat wj-Mcat merged commit 626db52 into PaddlePaddle:develop Dec 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants