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

[Example]【快乐开源】迁移 CFDGCN 案例至 PaddleScience #609

Merged
merged 10 commits into from
Dec 11, 2023

Conversation

MayYouBeProsperous
Copy link
Contributor

@MayYouBeProsperous MayYouBeProsperous commented Oct 30, 2023

PR types

Others

PR changes

Others

Describe

Copy link

paddle-bot bot commented Nov 2, 2023

Thanks for your contribution!

Copy link

paddle-bot bot commented Nov 2, 2023

✅ This PR's description meets the template requirements!
Please wait for other CI results.

@MayYouBeProsperous MayYouBeProsperous changed the title [WIP]【快乐开源】迁移 CFDGCN 案例至 PaddleScience [Example]【快乐开源】迁移 CFDGCN 案例至 PaddleScience Nov 29, 2023
@@ -119,6 +121,8 @@ def __init__(

self.nodes = self.mesh_graph[0]
self.edges = self.mesh_graph[1]
if edges_tran:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

amgnet和本模型数据集相同,但图格式不太一样吗

Copy link
Collaborator

Choose a reason for hiding this comment

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

看了下AMGNet的代码,这里的转置主要是为了后续在获取sender、receiver以及模型里涉及edge_index的切片更方便。这里可以加这么一个参数,但是名字改一下吧,叫transpose_edges,然后Docstring:Whether transpose the edges array from (2, num_edges) to (num_edges, 2) for convenient of slicing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Nov 29, 2023
Comment on lines 1 to 3
from .su2_function import SU2Module # noqa: F401
from .su2_function_mpi import activate_su2_mpi # noqa: F401
from .su2_numpy import SU2Numpy # noqa: F401
Copy link
Collaborator

Choose a reason for hiding this comment

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

去掉 from 后面的.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 23 to 50
r"""Pad a list of variable length Tensors with ``padding_value``
``pad_sequence`` stacks a list of Tensors along a new dimension,
and pads them to equal length. For example, if the input is list of
sequences with size ``L x *`` and if batch_first is False, and ``T x B x *``
otherwise.
`B` is batch size. It is equal to the number of elements in ``sequences``.
`T` is length of the longest sequence.
`L` is length of the sequence.
`*` is any number of trailing dimensions, including none.
Example:
>>> a = paddle.ones(25, 300)
>>> b = paddle.ones(22, 300)
>>> c = paddle.ones(15, 300)
>>> pad_sequence([a, b, c]).shape
paddle.Tensor([25, 3, 300])
Note:
This function returns a Tensor of size ``T x B x *`` or ``B x T x *``
where `T` is the length of the longest sequence. This function assumes
trailing dimensions and type of all the Tensors in sequences are same.
Args:
sequences (list[Tensor]): list of variable length sequences.
batch_first (bool, optional): output will be in ``B x T x *`` if True, or in
``T x B x *`` otherwise
padding_value (float, optional): value for padded elements. Default: 0.
Returns:
Tensor of size ``T x B x *`` if :attr:`batch_first` is ``False``.
Tensor of size ``B x T x *`` otherwise
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. 不同段落之间加一个空行
  2. 两个连续的 `` 改为一个 `

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 70 to 73
# TODO (Hui Zhang): set_value op not supprot `end==start`
# TODO (Hui Zhang): set_value op not support int16
# TODO (Hui Zhang): set_varbase 2 rank not support [0,0,...]
# out_tensor[i, :length, ...] = tensor
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个注释是paddle框架目前不支持吗?如果不是可以删除?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的已删除

Comment on lines 79 to 80
# TODO (Hui Zhang): set_value op not supprot `end==start`
# out_tensor[:length, i, ...] = tensor
Copy link
Collaborator

Choose a reason for hiding this comment

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

同上

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

scale_each: bool = False,
pad_value: int = 0,
**kwargs,
) -> paddle.Tensor:
Copy link
Collaborator

Choose a reason for hiding this comment

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

完善docstring

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这部分代码未使用已删除



def write_graph_mesh(
output_filename: Union[str, PathLike],
Copy link
Collaborator

Choose a reason for hiding this comment

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

所有路径只使用 str, 不要引入额外的库增加理解成本

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的已修改

# graph.tensor()
# graph.shape = [len(batch)]
# return graph
batch = np.array([g.tensor() for g in batch])
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个改动没看懂,g.tensor()返回的是内部数据转成tensor的graph,然后又转成np.array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

np.array 中的元素是graph对象,本来想返回 list,但下面代码会报错。

total_batch_size += next(iter(input_dict.values())).shape[0]

Copy link
Collaborator

Choose a reason for hiding this comment

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

np.array 中的元素是graph对象,本来想返回 list,但下面代码会报错。

total_batch_size += next(iter(input_dict.values())).shape[0]

你说得对,这里我觉得使用 len(next(iter(input_dict.values()))) 会更好,这样能兼容非Tensor类型的数据。

self.output_keys = output_keys
meshes_temp_dir = "temp_meshes"
os.makedirs(meshes_temp_dir, exist_ok=True)
self.mesh_file = meshes_temp_dir + "/" + str(os.getpid()) + "_mesh.su2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

所有涉及路径的拼接一律使用os.path.join,不使用加号

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 282 to 284
self.write_mesh_file(
nodes, self.elems, self.marker_dict, filename=self.mesh_file
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

每次前向都往磁盘里写数据这个操作是必要的吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

确实不用,移到了 init 中

@@ -119,6 +121,8 @@ def __init__(

self.nodes = self.mesh_graph[0]
self.edges = self.mesh_graph[1]
if edges_tran:
Copy link
Collaborator

Choose a reason for hiding this comment

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

看了下AMGNet的代码,这里的转置主要是为了后续在获取sender、receiver以及模型里涉及edge_index的切片更方便。这里可以加这么一个参数,但是名字改一下吧,叫transpose_edges,然后Docstring:Whether transpose the edges array from (2, num_edges) to (num_edges, 2) for convenient of slicing.

Copy link
Collaborator

@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.

现有改动可能会导致AMGNet的训练、评估无法跑通

# linux
wget https://paddle-org.bj.bcebos.com/paddlescience/datasets/AMGNet/data.zip
# windows
# curl https://paddle-org.bj.bcebos.com/paddlescience/datasets/AMGNet/data.zip --output data.zip
# unzip it
unzip data.zip
python amgnet_cylinder.py mode=eval EVAL.pretrained_model_path=https://paddle-org.bj.bcebos.com/paddlescience/models/amgnet/amgnet_cylinder_pretrained.pdparams

因为把collate_fn从合并graph改成了不合并graph,需要把AMGNet的forward、案例代码里的eval_expr和loss_expr内,对graph list进行手动合并

Copy link
Collaborator

Choose a reason for hiding this comment

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

代码文件开头加一下 LICENSE 文本

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

# graph.tensor()
# graph.shape = [len(batch)]
# return graph
batch = np.array([g.tensor() for g in batch])
Copy link
Collaborator

Choose a reason for hiding this comment

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

np.array 中的元素是graph对象,本来想返回 list,但下面代码会报错。

total_batch_size += next(iter(input_dict.values())).shape[0]

你说得对,这里我觉得使用 len(next(iter(input_dict.values()))) 会更好,这样能兼容非Tensor类型的数据。

@MayYouBeProsperous
Copy link
Contributor Author

MayYouBeProsperous commented Dec 8, 2023

CFDGCN 的 forward 中也会合并图,所以还是将 collate_fn 保持原样,转而修改 CFDGCN ,这样代码更整洁。

Copy link
Collaborator

@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

@HydrogenSulfate
Copy link
Collaborator

CFDGCN 的 forward 中也会合并图,所以还是将 collate_fn 保持原样,转而修改 CFDGCN ,这样代码更整洁。

辛苦大佬

@MayYouBeProsperous
Copy link
Contributor Author

MayYouBeProsperous commented Dec 10, 2023

案例文档还在编写中

@HydrogenSulfate HydrogenSulfate merged commit e14c71d into PaddlePaddle:develop Dec 11, 2023
3 of 4 checks passed
huohuohuohuohuo123 pushed a commit to huohuohuohuohuo123/PaddleScience that referenced this pull request Aug 12, 2024
* add cdfgcn mode

* add cfdgcn example

* code style

* fix su2paddle

* refine code

* codestyle

* fix bugs and add visualization

* fix

* fix cfdgcn model
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor HappyOpenSource 快乐开源活动issue与PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants