[BUG]fix bugs in make_sdf function - #1263
Conversation
Sorry, something went wrong.
|
Thanks for your contribution! |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
| ) | ||
| return sdf, sdf_derives | ||
| if grad is not None: | ||
| grad = grad.numpy().reshape(-1, 3) |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
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:
- Compute the direction vector: grad_vec = grad.numpy().reshape(-1, 3) - pts
- 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.
| 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 |
Sorry, something went wrong.
|
@shaunxq 非常感谢你提交修复BUG的PR，看你的修改是针对SDFMesh这个类的，但是你PR描述里说的却是ppsci.geometry.Geometry.sample_interior这个基类的方法。SDFMesh在创建的时候，它的vectors是固定的。所以我没太明白说的存在BUG的情况，能举一个更具体的例子吗？ |
Sorry, something went wrong.
@HydrogenSulfate PR里描述的是发现问题的地方，sample_interior会多次随机采样直到数量达到n，然后再对整体计算一次sdf，随机采样内部也会计算sdf。 --------分割线-------- |
Sorry, something went wrong.
你的意思是说，这个缩放本身存在浮点数运算精度误差，然后导致边界附近的sdf的正负号会出问题是吗 |
Sorry, something went wrong.
是的 |
Sorry, something went wrong.
Reviewers
At least 1 approving review is required to merge this pull request.
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) ...