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

pre-calculated normals are slow in debug builds; lead to test timeouts in CI #22311

Open
rpoyner-tri opened this issue Dec 14, 2024 · 2 comments
Assignees
Labels
component: geometry proximity Contact, distance, signed distance queries and related properties priority: low type: bug

Comments

@rpoyner-tri
Copy link
Contributor

rpoyner-tri commented Dec 14, 2024

What happened?

#22303
#22297

I bisected on test run-time over memory_leak_test on my local machine. It never times out locally (local resources are faster than CI), but its execution time jumped from 12sec to 20sec after the signed-distance-for-meshes patch landed. Note well that this is only for debug-built code; so it's mostly developers that would inconvenienced.

Version

v1.36.0

What operating system are you using?

Ubuntu 22.04

What installation option are you using?

compiled from source code using Bazel

Relevant log output

No response

@rpoyner-tri rpoyner-tri added type: bug priority: low component: geometry proximity Contact, distance, signed distance queries and related properties labels Dec 14, 2024
@rpoyner-tri rpoyner-tri self-assigned this Dec 14, 2024
@rpoyner-tri
Copy link
Contributor Author

/cc @DamrongGuoy . Worth considering on-demand computation of normals, if it is not too complicated to do. Don't drop anything to work on this, but I'd appreciate any tips you may have.

Meanwhile, I may fiddle with the tests (reduce repetitions, add sharding) to make them less vulnerable to timeouts.

@DamrongGuoy
Copy link
Contributor

Do we happen to have profiling data, please?

About on-demand computation, yes, I think we can do it. The first call to ComputeSignedDistanceToPoint, we can fill in this table. That's all we need, I think.

https://github.com/RobotLocomotion/drake/blob/d819ff20933392a1a81b9b9ab0d218d078b20c63/geometry/proximity_engine.cc#L1223C1-L1224C73

  // Data for ComputeSignedDistanceToPoint from meshes (Mesh and Convex).
  std::unordered_map<GeometryId, MeshDistanceBoundary> mesh_sdf_data_{};
};

We might have to think a bit what to do when users change shape of geometries. We can fill in the table entry for that one geometry right away, I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: geometry proximity Contact, distance, signed distance queries and related properties priority: low type: bug
Projects
None yet
Development

No branches or pull requests

2 participants