Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
【Hackathon No.95】 #61
【Hackathon No.95】 #61
Changes from 1 commit
f5e41d9
c79ec94
67d2ca6
668e472
a9e7543
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
如果是当一整个黑盒的话,这里大概后面不需要再Chain了?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里是有点奇怪,因为我在使用PyCallChainRules.jl和NeuralPDE结合时,如果不把jlwrap再用一层
Chain
包装起来的话,下面一句DiffEqFlux.initial_params(jlwrap)
得到的就会是一句空数组,而如果用Chain
包装起来就可以得到预期的参数数组There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
但是如果是调用
Optimisers.destructure
的话,直接传入jlwrap是可行的,之后我会再考虑用哪种形式There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
是的,得实现下这个的
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
因为这一步主要只是得到一个flatten后的数组,倾向于在实现NeuralPDE的例子时直接调用
Optimisers.destructure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
感觉这个constructor不是特别有必要,因为这里是specialized for dense layer,这里换个更具体的名字好一些
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
感谢建议,已进行了修订
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这部分能否对最后验收的内容描述得更具体一些?比如,后面是像 PyCallChainRules一样提供一个迷你的package,还是说只是以一般demo源码形式提供呢?
个人倾向于前者,这样也好方便后面其他人复现和优化 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
因为有部分实现是和PyCallChianRules.jl重合的,是否可以考虑以在PyCallChianRules.jl上添加模块的形式,或者先完成这部分的任务,再作为后续的整合工作
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
嗯,如果 PyCallChainRules.jl 里的接口有不够灵活的地方可以顺手去发个pr,cc我下,我们可以一起看看