Skip to content

[WIP] Fix SDF behavior of ppsci.geometry.Mesh and SDFMesh - #1270

Closed
HydrogenSulfate with Copilot wants to merge 1 commit into
developfrom
copilot/fix-sdf-behavior-ppsci-geometry
Closed

[WIP] Fix SDF behavior of ppsci.geometry.Mesh and SDFMesh#1270
HydrogenSulfate with Copilot wants to merge 1 commit into
developfrom
copilot/fix-sdf-behavior-ppsci-geometry

Conversation

Copilot AI commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.

Create a comprehensive, regression-tested fix for the SDF behavior of ppsci.geometry.Mesh / SDFMesh, taking PR #1263 as the starting point but addressing hidden correctness problems rather than copying it blindly.

Context

PR #1263 ([BUG]fix bugs in make_sdf function) correctly identifies that make_sdf(vectors) currently derives its normalization origin and scale from the query batch. This makes the result dependent on which points are evaluated together. SDFMesh.random_points() calls is_inside() repeatedly on individual batches, then sample_interior() evaluates SDF again over the assembled sample; near the surface, numerical differences can therefore make accepted interior points yield positive signed distance / negative sdf weights after the framework negates SDF.

The current problematic implementation is in ppsci/geometry/mesh.py, make_sdf near the bottom of the file. Relevant consumers include:

  • SDFMesh.from_stl, translate, and scale, which construct an SDF closure with make_sdf.
  • SDFMesh.is_inside, which relies on self.sdf_func(x) < 0.
  • SDFMesh.sample_interior, which negates SDF to create positive weights and optionally returns derivatives.
  • Boolean operations (union, difference, intersection), which compose SDF closures and gradients.

Required implementation goals

  1. Make SDF normalization invariant to query batching:

    • Calculate and capture mesh-derived normalization values once when make_sdf(vectors) is created.
    • Apply exactly the same fixed affine transform to every query point.
    • Preserve physical distance units by rescaling returned SDF values to the original coordinate scale.
    • Handle degenerate geometry safely: do not divide by zero if the mesh has zero extent. Prefer a clear ValueError at construction time, unless the repository’s established behavior/tests justify another approach.
  2. Preserve the actual signed-distance/gradient contract:

    • Confirm the expected input and return types of ppsci.geometry.sdf.signed_distance_field; do not introduce list conversions unless they are required by the checked-in implementation/API.
    • With compute_sdf_derivatives=True, calculate a finite, correctly oriented unit gradient in original/world coordinate space from query point and closest/hit point.
    • Avoid NaN/Inf on points exactly on the surface. Define and test the chosen behavior (for example zero gradient for zero distance), and ensure it is consistent with the existing sample_interior negation convention.
    • Do not mix normalized flattened point coordinates with hit points when computing gradients.
  3. Ensure behavior is compatible with transforms and CSG:

    • Validate SDF sign/distance and batch invariance for a simple watertight mesh (e.g. cube/tetrahedron) constructed in tests without external STL files where feasible.
    • Validate that translated and uniformly scaled SDFMesh instances still have correct SDF signs and distance scaling.
    • Validate at least one composed SDF operation if it can be tested without fragile third-party dependencies.
  4. Add focused regression tests:

    • A point evaluated alone, in one batch, and as part of a larger/differently distributed batch must return equal SDF (within numerical tolerance).
    • Points accepted by SDFMesh.random_points / returned by sample_interior must not produce materially negative final "sdf" weights. Use an appropriate small numerical tolerance near the boundary, rather than asserting exact non-negativity if the SDF backend has floating-point noise.
    • Test gradient finiteness and orientation if SDF backend functionality supports hit points in the test environment.
    • Tests should be deterministic and avoid adding heavyweight optional dependencies where possible. Follow the project’s existing test layout/conventions; if SDF dependencies are optional, use existing skip patterns or isolate pure helper logic suitably.
  5. Maintain project style and public API compatibility. Update docstrings/comments only where necessary to clarify the fixed-coordinate normalization and derivative convention.

Review notes / likely hidden issues to specifically investigate

  • PR [BUG]fix bugs in make_sdf function #1263 changes the gradient calculation from an opaque expression to pts - hit_points. Verify orientation rather than assuming it: SDFMesh.sample_interior separately negates derivatives, and the sign must agree with that contract.
  • The PR’s conversion of mesh vertices / points to list[tuple] may be unnecessary or may degrade performance/API compatibility; use the type expected by the local SDF wrapper.
  • The PR’s norms[norms < 1e-6] = 1.0 avoids division by zero but leaves the original nonzero vector divided by 1 for near-surface values; choose behavior intentionally and test it.
  • Mesh vertex arrays may contain repeated vertices; this must not affect normalization or SDF triangle indexing.

Validation

Run the narrow added/affected tests and any relevant formatting/lint checks available in the repository. In the PR description, clearly state the original batch-dependent normalization defect, the additional edge cases fixed, and tests executed.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: 请帮忙判断下这个修复是否是合理的

Assistant: 我来帮你审查这个PR的合理性。首先让我查看一下PR的具体改动内容。

Assistant: Let me fetch the actual changes from the PR:

Assistant: 现在让我查看 sample_interior 函数的实现,以及 SDF 的使用方式:

User: 请你继续

Assistant: 现在我已经理解了这个修复的核心问题。让我详细分析:

问题分析

修复是合理的

这个修复针对的是 make_sdf 函数中的一个精度一致性问题,修复思路正确。


问题根源

旧代码的缺陷(第 1362-1404 行,main 分支)

def make_sdf(vectors: np.ndarray):
    def sdf_func(points: np.ndarray, compute_sdf_derivatives=False):
        # ❌ 问题:根据每批采样点动态调整边界
        points = points.copy()
        x_min, y_min, z_min = np.min(points, axis=0)  # 基于输入点
        x_max, y_max, z_max = np.max(points, axis=0)
        max_dis = max(max((x_max - x_min), (y_max - y_min)), (z_max - z_min))
        
        # 同时调整三角形和点
        store_triangles = vectors.copy()
        store_triangles[:, :, 0] -= x_min
        # ... 其他调整

核心问题

  1. 每次调用 sdf_func(points) 时,归一化的参考坐标系都可能不同
  2. 某个内部点在一批数据中被判定为内部(sdf < 0
  3. 但在另一批数据中可能被判定为外部(sdf > 0),因为归一化边界不同
  4. 这导致同一点的 SDF 值符号可能反转,内部约束损失出现负数

新代码的修复(PR 改动)

def make_sdf(vectors: np.ndarray):
    # ✅ 在创建 sdf_func 闭包时,就固定好几何的边界
    verts = vectors.reshape(-1, 3)
    v_min = verts.min(axis=0)
    v_max = verts.max(axis=0)
    max_dis = np.max(v_max - v_min)
    norm_mesh = (verts - v_min) / max_dis
    
    def sdf_func(points: np.ndarray, compute_sdf_derivatives=False):
        # ✅ 使用固定的网格和归一化参数
        pts = (points - v_min) / max_dis  # 使用原始几何的边界
        # ...

优势

  • 所有采样点都基于原始几何的边界进行归一化
  • 保证一致性:同一点的 SDF 值永远不变
  • 解决内部点判定不一致的问题

代码质量评估

优点

  1. 逻辑清晰:将归一化从动态计算改为静态预计算
  2. 修复准确:直击问题根源
  3. 代码简化:移除了冗余的 copy 和动态计算,性能更优

需要注意的地方 ⚠️

在 PR 的梯度计算部分有改动:

旧代码(第 1393-1399 行):

sdf_derives = sdf_derives.numpy().reshape(-1)
sdf_derives = -(sdf_derives - points)  # ← 这里的逻辑有问题
sdf_derives = np.reshape(sdf_derives, (sdf_derives.shape[0] // 3, 3))
sdf_derives = sdf_derives / np.linalg.norm(sdf_derives, axis=1, keepdims=True)

新代码(PR 改动):

hit_points = hit_points.numpy().reshape(-1, 3)
grad_vec = pts - hit_points  # ✅ 更清晰:点到最近表面点的向量
norms = np.linalg.norm(grad_vec, axis=1, keepdims=True)
norms[norms < 1e-6] = 1.0  # ✅ 处理数值稳定性
grad = grad_vec / norms

这部分改动:

  • 更正确:使用 hit_points 而不是复杂的减法
  • 更稳定:处理了零向量的情况(norms < 1e-6
  • 更清晰:梯度的几何意义更明显

建议

✅ 这个修复应该被合并

但建议检查以下几点:

  1. 测试覆盖:确保有单元测试验证

    • 相同点多次计算 SDF 结果一致
    • 内部点的 SDF 值始终为负
    • 外部点的 SDF 值始终为正
  2. 梯度计算验证:确认数值梯度与 SDF 梯度的对应关系

  3. 向后兼容:检查是否有其他代码依赖旧的动态边界行为

这是一个有必要的 bug 修复,核心思想是用一致的参考坐标系替代动态变化的参考坐标系

User: 这个PR本身是否有其他隐蔽的问题

User: 请你基于这个PR和上述判断,帮我重新提交一个PR,彻底修复下Mesh类存在的问题

Assistant: 我已经准备好创建一个新的 PR(基于 develop),包含对 SDFMesh 批处理无关 SDF、零尺度网格、梯度方向/零距离稳定性,以及回归测试的完整修复;目前正在等待你确认启动该代码修改任务。

@CLAassistant

Copy link
Copy Markdown

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.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants