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

WIP: lazy union #100

Closed
wants to merge 4 commits into from
Closed

WIP: lazy union #100

wants to merge 4 commits into from

Conversation

pca006132
Copy link
Collaborator

@pca006132 pca006132 commented May 1, 2022

Implemented ManifoldSet, which is a set of manifolds that are unioned together.

This allows users to union manifolds lazily, and use compose as much as possible for faster union. This reduces the time required to union a large set of manifolds significantly when each of them only intersect with a small number of neighbors. Most importantly, this allows users to incrementally compose manifolds together and only pay for the Manifold::Impl::Finish once, which is expensive. A simple cache is implemented using std::shared_ptr to allow multiple ManifoldSet to share the same set of manifolds if they only differ in transformation (this cache is not thread safe, but this can be changed if needed).

A simple union benchmark is implemented. It will union 100 spheres (with circularSegments=60 to simulate complex objects) with radius 2.5 with different distances.

Benchmark results:

CUDA:

Separation: 0
Union:        took 1.07531 seconds
Manifold Set: took 1.08462 seconds
----------------------
Separation: 1
Union:        took 1.41711 seconds
Manifold Set: took 0.968532 seconds
----------------------
Separation: 2
Union:        took 2.20316 seconds
Manifold Set: took 0.926535 seconds
----------------------
Separation: 3
Union:        took 3.278 seconds
Manifold Set: took 0.984004 seconds
----------------------
Separation: 4
Union:        took 4.69098 seconds
Manifold Set: took 1.34924 seconds
----------------------
Separation: 5
Union:        took 5.90308 seconds
Manifold Set: took 1.65334 seconds
----------------------
Separation: 6
Union:        took 5.49614 seconds
Manifold Set: took 0.832142 seconds
----------------------
Separation: 7
Union:        took 5.50061 seconds
Manifold Set: took 0.844545 seconds
----------------------
Separation: 8
Union:        took 5.47394 seconds
Manifold Set: took 0.835328 seconds
----------------------
Separation: 9
Union:        took 5.48175 seconds
Manifold Set: took 0.836789 seconds

OpenMP:

Separation: 0
Union:        took 1.51363 seconds
Manifold Set: took 1.55817 seconds
----------------------
Separation: 1
Union:        took 1.3737 seconds
Manifold Set: took 0.95533 seconds
----------------------
Separation: 2
Union:        took 2.34161 seconds
Manifold Set: took 1.02168 seconds
----------------------
Separation: 3
Union:        took 3.63707 seconds
Manifold Set: took 1.13614 seconds
----------------------
Separation: 4
Union:        took 5.41142 seconds
Manifold Set: took 1.59267 seconds
----------------------
Separation: 5
Union:        took 7.04236 seconds
Manifold Set: took 1.96182 seconds
----------------------
Separation: 6
Union:        took 6.76521 seconds
Manifold Set: took 1.02406 seconds
----------------------
Separation: 7
Union:        took 6.73354 seconds
Manifold Set: took 1.01433 seconds
----------------------
Separation: 8
Union:        took 6.72667 seconds
Manifold Set: took 1.02749 seconds
----------------------
Separation: 9
Union:        took 6.71657 seconds
Manifold Set: took 1.02329 seconds

This allows users to union manifolds lazily, and use compose as much as
possible for faster union. This reduces the time required to union
a large set of manifolds significantly when each of them only intersect
with a small number of neighbours.
@pca006132 pca006132 changed the title lazy union WIP: lazy union May 2, 2022
@pca006132 pca006132 marked this pull request as draft May 2, 2022 14:29
@pca006132
Copy link
Collaborator Author

Something weird is going on here...

pca006132@a508be4#diff-cac39eea04c29dc7f2011594bfa3d917a7ef8446598742862f0ba71ee19e67a9L42

It was working on another repository (no python binding there, I just wrote a very simple communication via stdin/stdout) before this patch, but it stops working after this... Just in case anyone is interested in debugging this, here is the python script: https://github.com/pca006132/manifold/blob/pybind11/test.py (this branch contains the python binding using this lazy union)

@pca006132
Copy link
Collaborator Author

I guess I will need some help for this. This works if I change the transform function from

ManifoldSet ManifoldSet::Transform(const glm::mat4x3 &transform) const {
  ManifoldSet result;
  result.manifolds = manifolds;
  result.transform_ = transform * glm::mat4(transform_);
  return result;
}

to

ManifoldSet ManifoldSet::Transform(const glm::mat4x3 &transform) const {
  ManifoldSet result;
  result.manifolds->push_back(ToManifold().Transform(transform));
  return result;
}

but does not work even if I do

ManifoldSet ManifoldSet::Transform(const glm::mat4x3 &transform) const {
  ManifoldSet result;
  // copy the manifolds and apply transformation
  for (auto m : *manifolds) {
    result.manifolds->push_back(m);
    result.manifolds->back().Transform(transform);
  }
  return result;
}

(test using the python script in https://github.com/pca006132/manifold/blob/pybind11/test.py)

@pca006132
Copy link
Collaborator Author

Closing this as this will no longer be implemented as a separate class, and the design will be quite different from this.

@pca006132 pca006132 closed this May 16, 2022
@pca006132 pca006132 deleted the manifold-set branch December 22, 2022 05:35
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.

None yet

1 participant