Skip to content

[BUG]fix bugs in make_sdf function#1263

Open
shaunxq wants to merge 2 commits intoPaddlePaddle:developfrom
shaunxq:dev_sdf
Open

[BUG]fix bugs in make_sdf function#1263
shaunxq wants to merge 2 commits intoPaddlePaddle:developfrom
shaunxq:dev_sdf

Conversation

@shaunxq
Copy link

@shaunxq shaunxq commented Feb 2, 2026

PR types

Bug fixes

PR changes

Others

Describe

1.原始的sdf_func会根据每批采样点动态调整边界,导致ppsci.geometry.Geometry.sample_interior函数采样内部点的时候,由于精度问题,sdf判断不一致:某批random_points判断为内部,整体计算sdf又被判定为外部。出现内部采样点的sdf距离存在负数的情况,最终表现为内部约束损失在多轮迭代后变成负数。这里统一改为采用原始几何来计算边界。

class Geometry:
    ...

    def sample_interior(...) -> Dict[str, np.ndarray]:
            ...

                if misc.typename(self) == "TimeXGeometry":
                    points = self.random_points(n, random, criteria)
                else:
                    points = self.random_points(n, random)

            ...

        # if sdf_func added, return x_dict and sdf_dict, else, only return the x_dict
        if hasattr(self, "sdf_func"):
            # NOTE: add negative to the sdf values because weight should be positive.
            sdf = -self.sdf_func(x)
           
            ...

@paddle-bot
Copy link

paddle-bot bot commented Feb 2, 2026

Thanks for your contribution!

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


xiangqian seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a critical bug in the make_sdf function where the signed distance field calculation dynamically adjusted boundaries based on each batch of sampled points, causing inconsistencies in interior point sampling. The fix changes the implementation to use fixed boundaries based on the original mesh geometry, ensuring consistent SDF calculations across all point batches.

Changes:

  • Modified make_sdf to compute mesh bounds once at initialization (v_min, v_max, max_dis) instead of dynamically per point batch
  • Changed normalization to use fixed mesh-based bounds for both mesh vertices and query points
  • Updated API calls to use list of tuples format (required by warp SDF module) instead of flattened numpy arrays
  • Simplified gradient handling by directly using returned values from SDF module

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

)
return sdf, sdf_derives
if grad is not None:
grad = grad.numpy().reshape(-1, 3)
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

The gradient calculation is incorrect. The variable grad is the hit point (closest point on mesh surface) returned from the warp SDF module, not the gradient itself. The gradient of the signed distance field should be the normalized vector from the query point to the closest surface point.

The current code returns the hit point coordinates directly, but it should compute the direction vector and normalize it. The correct implementation should be:

  1. Compute the direction vector: grad_vec = grad.numpy().reshape(-1, 3) - pts
  2. Normalize it: grad = grad_vec / np.linalg.norm(grad_vec, axis=1, keepdims=True)

This matches the implementation in the SDFMesh.sdf_func method (lines 826-828) which correctly computes gradients as normalized vectors from query points to hit points.

Suggested change
grad = grad.numpy().reshape(-1, 3)
# `grad` from the SDF module are hit points in normalized coordinates.
# Convert them into normalized gradient directions from query points
# to the closest surface points.
hit_points = grad.numpy().reshape(-1, 3)
grad_vec = hit_points - pts
norms = np.linalg.norm(grad_vec, axis=1, keepdims=True)
# Avoid division by zero for degenerate cases
norms[norms == 0] = 1.0
grad = grad_vec / norms

Copilot uses AI. Check for mistakes.
@HydrogenSulfate
Copy link
Collaborator

@shaunxq 非常感谢你提交修复BUG的PR,看你的修改是针对SDFMesh这个类的,但是你PR描述里说的却是ppsci.geometry.Geometry.sample_interior这个基类的方法。SDFMesh在创建的时候,它的vectors是固定的。所以我没太明白说的存在BUG的情况,能举一个更具体的例子吗?

@shaunxq
Copy link
Author

shaunxq commented Feb 4, 2026

@shaunxq 非常感谢你提交修复BUG的PR,看你的修改是针对SDFMesh这个类的,但是你PR描述里说的却是ppsci.geometry.Geometry.sample_interior这个基类的方法。SDFMesh在创建的时候,它的vectors是固定的。所以我没太明白说的存在BUG的情况,能举一个更具体的例子吗?

@HydrogenSulfate PR里描述的是发现问题的地方,sample_interior会多次随机采样直到数量达到n,然后再对整体计算一次sdf,随机采样内部也会计算sdf。
SDFMesh的vectors是固定的,因为原始逻辑是动态归一化,根据输入的采样点的包围盒对vectors进行缩放,某批随机采样的包围盒和整体的包围盒存在一定偏差,导致某个随机采样判断是内部的点,整体计算的时候被判断为外部,sdf就变成了负数,sdf权重损失就成了负数。

--------分割线--------
AI检查部分导数那块第一次提交存在问题,遗漏了,重新提交了修改

@HydrogenSulfate
Copy link
Collaborator

@shaunxq 非常感谢你提交修复BUG的PR,看你的修改是针对SDFMesh这个类的,但是你PR描述里说的却是ppsci.geometry.Geometry.sample_interior这个基类的方法。SDFMesh在创建的时候,它的vectors是固定的。所以我没太明白说的存在BUG的情况,能举一个更具体的例子吗?

@HydrogenSulfate PR里描述的是发现问题的地方,sample_interior会多次随机采样直到数量达到n,然后再对整体计算一次sdf,随机采样内部也会计算sdf。 SDFMesh的vectors是固定的,因为原始逻辑是动态归一化,根据输入的采样点的包围盒对vectors进行缩放,某批随机采样的包围盒和整体的包围盒存在一定偏差,导致某个随机采样判断是内部的点,整体计算的时候被判断为外部,sdf就变成了负数,sdf权重损失就成了负数。

--------分割线-------- AI检查部分导数那块第一次提交存在问题,遗漏了,重新提交了修改

你的意思是说,这个缩放本身存在浮点数运算精度误差,然后导致边界附近的sdf的正负号会出问题是吗

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