Skip to content

use scikit instead of torchmcubes - #26

Closed
flowtyone wants to merge 1 commit into
VAST-AI-Research:mainfrom
flowtyone:use-scikit
Closed

use scikit instead of torchmcubes#26
flowtyone wants to merge 1 commit into
VAST-AI-Research:mainfrom
flowtyone:use-scikit

Conversation

@flowtyone

Copy link
Copy Markdown

Increase usability by using scikit-image instead of the old torchmcubes. This makes the project much easier to install and test.

@mrbid

mrbid commented Mar 7, 2024

Copy link
Copy Markdown

Increase usability by using scikit-image instead of the old torchmcubes. This makes the project much easier to install and test.

Just out of interest are there any obvious quality benefits over using scikit-image or is this mostly just an installing and testing solution? Speed benefits, wider range of GPU support etc? It would be interesting to actually see a full comparison between the two.

@flowtyone

Copy link
Copy Markdown
Author

@mrbid speed wasn't impacted in my tests. The issue with torchmcubes is that it lacks prebuilt wheels and has a dependency issue with torch that makes it harder to install

@math-sasso

Copy link
Copy Markdown

@flowtyone how did you actually solved the problem? If you share a step by step would help a lot. I am struggling a lot to make it work

@flowtyone

Copy link
Copy Markdown
Author

@math-sasso this PR works, just use my fork branch, it has these changes

@mrbid

mrbid commented Mar 28, 2024

Copy link
Copy Markdown

https://github.com/flowtyone/TripoSR/commit/341657e34f98f314cdfdcfbabe689d0d52dde7a5

It's quite a minimal and elegant patch. No need to even fork it tbh. This should be merged as an optional function in TripoSR, allowing the user to select which is used - ofc that wont solve the dependency issue, and I don't know if there is a similar ifdef system in Python as there is in C.

@mrbid

mrbid commented Apr 4, 2024

Copy link
Copy Markdown

I have an error with your fork:

Traceback (most recent call last):
  File "/home/r/Desktop/TripoSR-use-scikit/run.py", line 32, in <module>
    mesh = model.extract_mesh(scene_codes, resolution=256)[0]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/r/Desktop/TripoSR-use-scikit/tsr/system.py", line 172, in extract_mesh
    self.set_marching_cubes_resolution(resolution)
  File "/home/r/Desktop/TripoSR-use-scikit/tsr/system.py", line 169, in set_marching_cubes_resolution
    self.isosurface_helper = MarchingCubeHelper(resolution)
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/r/Desktop/TripoSR-use-scikit/tsr/models/isosurface.py", line 20, in __init__
    self.mc_func: Callable = marching_cubes
                             ^^^^^^^^^^^^^^
NameError: name 'marching_cubes' is not defined

I was going to bench it against the original and this modified version by @thatname here: #22 (comment)

@TimCabbage

Copy link
Copy Markdown

@mrbid @flowtyone
To make it work, one more change is needed:
the line:
self.mc_func: Callable = marching_cubes
Needs to be removed.

Works on ROCM with this change.

@flowtyone flowtyone closed this by deleting the head repository May 7, 2025
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.

4 participants