Skip to content

Conversation

MayYouBeProsperous
Copy link
Contributor

No description provided.

Copy link

paddle-bot bot commented Mar 25, 2024

你的PR提交成功,感谢你对开源项目的贡献!
请检查PR提交格式和内容是否完备,具体请参考示例模版
Your PR has been submitted. Thanks for your contribution!
Please check its format and content. For this, you can refer to Template and Demo.

Copy link
Contributor

@HydrogenSulfate HydrogenSulfate left a comment

Choose a reason for hiding this comment

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

感谢提交PR,辛苦看一下comment

Comment on lines 72 to 78
2. 模型构建

在 `ppsci.arc` 中实现 `XPINN` 模型,并用以下形式调用模型。

```python
model = ppsci.arch.XPINN
```
Copy link
Contributor

@HydrogenSulfate HydrogenSulfate Mar 26, 2024

Choose a reason for hiding this comment

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

XPINN本身应该是一种扩展性很强的算法,跟arch绑定感觉不太好,可以考虑参考modulus的做法:https://github.com/NVIDIA/modulus-sym/blob/main/examples/ldc/ldc_2d_domain_decomposition.py

Copy link
Contributor Author

@MayYouBeProsperous MayYouBeProsperous Mar 29, 2024

Choose a reason for hiding this comment

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

好的,有个疑问,需要把 xpinn 算法模块化成 api 吗,还是只在 examples 中实现呢?

Copy link
Contributor

@HydrogenSulfate HydrogenSulfate Mar 29, 2024

Choose a reason for hiding this comment

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

好的,有个疑问,需要把 xpinn 算法模块化成 api 吗,还是只在 examples 中实现呢?

可以是API或者是像modulus这种具备快速扩展到其他案例上的代码块。前者作为API的话,可以考虑归类在PDE模块下,后者可以是像modulus里的一个代码块(函数)。如果做成API的话,可能需要考虑一下怎么设计才是容易扩展又符合正常使用逻辑的,如果是后者,那就只要写一个函数就行了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改,辛苦 review

Copy link
Contributor

@HydrogenSulfate HydrogenSulfate left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants