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

【PPSCI Doc No.66-74】 #829

Merged
merged 12 commits into from
Apr 4, 2024
Merged

Conversation

1want2sleep
Copy link
Contributor

PR types

Others

PR changes

Docs

Describe

补充了以下API的文档,包括功能介绍、输入参数描述、返回值描述、代码示例
ppsci.geometry.TimeDomain.on_initial
ppsci.geometry.TimeXGeometry.uniform_points
ppsci.geometry.TimeXGeometry.random_points
ppsci.geometry.TimeXGeometry.uniform_boundary_points
ppsci.geometry.TimeXGeometry.random_boundary_points
ppsci.geometry.TimeXGeometry.uniform_initial_points
ppsci.geometry.TimeXGeometry.random_initial_points
ppsci.geometry.TimeXGeometry.periodic_point
ppsci.geometry.TimeXGeometry.sample_initial_interior

Copy link

paddle-bot bot commented Apr 1, 2024

Thanks for your contribution!

@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Apr 2, 2024
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.

多处类似的问题可以一起修改

@@ -71,6 +71,24 @@ def __init__(
self.num_timestamps = len(timestamps)

def on_initial(self, t):
"""Check if a specific time is on the initial time point
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

@1want2sleep 1want2sleep Apr 2, 2024

Choose a reason for hiding this comment

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

All Done.

"""Check if a specific time is on the initial time point

Args:
t (float): The time to be checked.
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里的t应该是一个np.ndarray

Examples:
>>> import paddle
>>> import ppsci
>>> paddle.set_default_dtype("float64")
Copy link
Collaborator

Choose a reason for hiding this comment

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

paddle没必要设置dtype,因为几何模块用的是numpy后端

Comment on lines 87 to 89
>>> check1 = geom.on_initial(0.00000001)
>>> check2 = geom.on_initial(0.6)
>>> print(check1, check2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

构造长度为5的np.ndarray,并且包含t0和非t0的元素用于检查,这样比测两次更简洁

@@ -71,6 +71,24 @@ def __init__(
self.num_timestamps = len(timestamps)

def on_initial(self, t):
Copy link
Collaborator

Choose a reason for hiding this comment

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

加上type hint: t: np.ndarray

boundary (bool): Indicates whether boundary points are included, default is True.

Returns:
Ndarray: a set of spatial-temporal coordinate points 'tx' that represent sample points evenly distributed within the spatial-temporal domain.
Copy link
Collaborator

Choose a reason for hiding this comment

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

使用np.ndarray代替Ndarray

@@ -112,9 +130,25 @@ def boundary_normal(self, x):

def uniform_points(self, n, boundary=True):
"""Uniform points on the spatial-temporal domain.

Note: Before using this method, you need to specify time_step or timestamps.(See Examples)
Copy link
Collaborator

Choose a reason for hiding this comment

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

可以在__init__内加上对time_step和timestamps的检查,然后这句话可以删除

>>> timedomain = ppsci.geometry.TimeDomain(0, 1)
>>> geom = ppsci.geometry.Rectangle((0, 0), (1, 1))
>>> time_geom = ppsci.geometry.TimeXGeometry(timedomain, geom)
>>> time_geom.timedomain.time_step = 0.001
Copy link
Collaborator

Choose a reason for hiding this comment

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

time_step建议在TimeDomain构造时传入设置

>>> time_geom = ppsci.geometry.TimeXGeometry(timedomain, geom)
>>> time_geom.timedomain.time_step = 0.001
>>> ts = time_geom.uniform_points(1000)

Copy link
Collaborator

Choose a reason for hiding this comment

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

打印一下ts的形状

@1want2sleep
Copy link
Contributor Author

多处类似的问题可以一起修改

Done.

@1want2sleep 1want2sleep closed this Apr 2, 2024
@1want2sleep 1want2sleep reopened this Apr 2, 2024
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.

感谢提交PR,有几处再略微修改一下即可

Comment on lines 154 to 155
if self.timedomain.time_step is None and self.timedomain.timestamps is None:
raise ValueError("Either time_step or timestamps must be provided.")
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.

All done.

x = self.geometry.random_points(n, random=random)
t = self.timedomain.t0
return np.hstack((np.full([n, 1], t, dtype=paddle.get_default_dtype()), x))

def periodic_point(self, x, component):
def periodic_point(self, x: dict[str, np.ndarray], component: int):
Copy link
Collaborator

Choose a reason for hiding this comment

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

dict类型的typehint统一使用 typing模块下的Dict

component (int): Specifies the components or dimensions of specific spatial coordinates that are periodically processed.

Returns:
dict[str, np.ndarray] : contains the original timestamps and the coordinates of the spatial point after periodic processing.
Copy link
Collaborator

Choose a reason for hiding this comment

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

同上,应该是大写的D

y (1000, 1)
normal_x (1000, 1)
normal_y (1000, 1)

Copy link
Collaborator

Choose a reason for hiding this comment

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

多余空行可以删除

@@ -544,11 +701,36 @@ def sample_initial_interior(
self,
n: int,
random: str = "pseudo",
criteria=None,
evenly=False,
criteria: callable = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

callable同理使用typing下的Callable

x (1000, 1)
y (1000, 1)
sdf (1000, 1)

Copy link
Collaborator

Choose a reason for hiding this comment

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

同上,删除空行

@@ -350,7 +423,31 @@ def uniform_boundary_points(self, n, criteria=None):
tx = tx[:n]
return tx

def random_boundary_points(self, n, random="pseudo", criteria=None):
def random_boundary_points(
self, n: int, random: str = "pseudo", criteria: callable = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. 使用typing下的Callable
  2. 默认值为None的参数需要用Optional嵌套在原类型上:Optional[Callble]

Args:
n (int): The total number of spatial-temporal points generated on a given geometry boundary.
random (string): Controls the way to generate random points. Default is "pseudo".
criteria (method): Used to filter the generated boundary points, only points that meet certain conditions are retained. Default is None.
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
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 HydrogenSulfate merged commit fcb277c into PaddlePaddle:develop Apr 4, 2024
3 of 4 checks passed
huohuohuohuohuo123 pushed a commit to huohuohuohuohuo123/PaddleScience that referenced this pull request Aug 12, 2024
* ppsci.equation.PDE.parameters/state_dict/set_state_dict api fix

* ppsci.equation.PDE.parameters/state_dict/set_state_dict api fix

* fix api docs in the timedomain

* fix api docs of timedomain

* fix api docs of timedomain

---------

Co-authored-by: krp <2934631798@qq.com>
Co-authored-by: HydrogenSulfate <490868991@qq.com>
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.

4 participants