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

Boolean unit tests fail if manifold3d is not installed #2306

Open
onitake opened this issue Oct 23, 2024 · 5 comments · Fixed by #2309
Open

Boolean unit tests fail if manifold3d is not installed #2306

onitake opened this issue Oct 23, 2024 · 5 comments · Fixed by #2309

Comments

@onitake
Copy link
Contributor

onitake commented Oct 23, 2024

Some of the unit tests have been set up so they execute with all available boolean mesh operation engines, but some of them require the default manifold3d engine.

The Debian build system requires that all dependencies must be available in the Debian ecosystem, but we haven't packaged manifold3d yet and need to run all unit tests against the blender engine for now.

Example error:

______________________________________________________________________________________________ test_multiple_difference _______________________________________________________________________________________________

    def test_multiple_difference():
        """
        Check that `a - b - c - d - e` does what we expect on both
        the base class method and the function call.
        """
    
        # make a bunch of spheres that overlap
        center = (
            np.array(
                [
                    [np.cos(theta), np.sin(theta), 0.0]
                    for theta in np.linspace(0.0, np.pi * 2, 5)
                ]
            )
            * 1.5
        )
        # first sphere is centered
        spheres = [g.trimesh.creation.icosphere()]
        spheres.extend(g.trimesh.creation.icosphere().apply_translation(c) for c in center)
    
        # compute using meshes method
>       diff_base = spheres[0].difference(spheres[1:])

test_boolean.py:223: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../trimesh/base.py:2926: in difference
    return boolean.difference(
../trimesh/boolean.py:47: in difference
    return _engines[engine](meshes, operation="difference", **kwargs)
../trimesh/boolean.py:137: in boolean_manifold
    mesh=Mesh(
../trimesh/exceptions.py:34: in __call__
    raise super().__getattribute__("exception")
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

    """
    boolean.py
    -------------
    
    Do boolean operations on meshes using either Blender or Manifold.
    """
    
    import numpy as np
    
    from . import exceptions, interfaces
    from .typed import Callable, NDArray, Optional, Sequence, Union
    
    try:
>       from manifold3d import Manifold, Mesh
E       ModuleNotFoundError: No module named 'manifold3d'

../trimesh/boolean.py:14: ModuleNotFoundError

Aside from test_multiple_difference, these tests would also need modifications: test_boolean_manifold, test_boolean_manifold, test_reduce_cascade

Maybe it would be a better idea to use a test fixture instead, so all unit tests are always run with all engines?
Or perhaps there should be a way to set the engine globally and autodetect available engines on module load.

@mikedh
Copy link
Owner

mikedh commented Oct 23, 2024

Does #2298 help at all? PR's welcome! One thing that might help breaking this in the future is if we added a test that tried the debian build, which I have no idea how to do but am open to PR's on 😄.

@onitake
Copy link
Contributor Author

onitake commented Oct 23, 2024

#2298 Is almost exactly what I patched to make the unit test no longer fail on the Debian build. 😅

I don't think there should be a Debian-specific test, but rather some better logic to select and test different engines.
For example, there is currently no automatic fallback to the blender engine when manifold3d isn't available, or vice-versa.
The rationale is that manifold3d is not a mandatory dependency of trimesh, so it shouldn't cause code to fail. Neither is Blender, of course. Boolean operations should only fail if no engine is available.

The unit tests should also run all tests against all engines and fail when an engine isn't available. In the Debian scripts, we can then simply add a small patch that removes manifold3d from the engine list in the test.

I'll try to come up with a PR based on the idea of selecting an engine globally on module load, with the option of changing the detected default later or passing it to each operation (as it's implemented now).

@mikedh
Copy link
Owner

mikedh commented Nov 1, 2024

The Debian build system requires that all dependencies must be available in the Debian ecosystem.

It looked like trimesh wasn't testing with blender at all (good catch!) so I was trying to add it to our docker images which use a Debian trixie base image. It looked like blender was either removed, or isn't pushed yet for trixie?

@onitake
Copy link
Contributor Author

onitake commented Nov 1, 2024

Yes, it's unfortunate, but it got kicked out automatically because some dependencies fail to build with other updated dependencies. It's a real mess lately... not least thanks to lots of changes/deprecations in Python 3.12 and 3.13.

You can see the status here: https://tracker.debian.org/pkg/blender

I recommend pulling the latest version from sid instead, that's what I usually do in these situations.

@mikedh
Copy link
Owner

mikedh commented Nov 5, 2024

No worries, I installed from a blender tarball. Let me know if #2312 needs some follow-ups on this issue.

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 a pull request may close this issue.

2 participants