gumyr / build123d

A python CAD programming library
Apache License 2.0
432 stars 81 forks source link

tests failing on M1 #478

Closed MatthiasJ1 closed 7 months ago

MatthiasJ1 commented 7 months ago
FAILED test_build_part.py::TestMakeBrakeFormed::test_make_brake_formed - OCP.Standard.Standard_Failure: BRep_API: command not done
FAILED test_direct_api.py::TestLocation::test_neg - AssertionError: -180.0 != 180.0 within 5 places (360.0 difference)
=========================================== FAILURES ============================================
__________________________ TestMakeBrakeFormed.test_make_brake_formed ___________________________

self = <test_build_part.TestMakeBrakeFormed testMethod=test_make_brake_formed>

    def test_make_brake_formed(self):
        # TODO: Fix so this test doesn't raise a DeprecationWarning from NumPy
        with BuildPart() as bp:
            with BuildLine() as bl:
                Polyline((0, 0), (5, 6), (10, 1))
>               fillet(bl.vertices(), 1)

test_build_part.py:62: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
build123d/operations_generic.py:463: in fillet
    new_wire = target.fillet_2d(radius, object_list)
build123d/topology.py:7056: in fillet_2d
    return Face.make_from_wires(self).fillet_2d(radius, vertices).outer_wire()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <build123d.topology.Face object at 0x13fcc9c40>, radius = 1
vertices = <filter object at 0x13fccb460>

    def fillet_2d(self, radius: float, vertices: Iterable[Vertex]) -> Face:
        """Apply 2D fillet to a face

        Args:
          radius: float:
          vertices: Iterable[Vertex]:

        Returns:

        """

        fillet_builder = BRepFilletAPI_MakeFillet2d(self.wrapped)

        for vertex in vertices:
            fillet_builder.AddFillet(vertex.wrapped, radius)

        fillet_builder.Build()

>       return self.__class__(fillet_builder.Shape())
E       OCP.Standard.Standard_Failure: BRep_API: command not done

build123d/topology.py:5570: Standard_Failure
_____________________________________ TestLocation.test_neg _____________________________________

self = <test_direct_api.TestLocation testMethod=test_neg>

    def test_neg(self):
        loc = Location((1, 2, 3), (0, 35, 127))
        n_loc = -loc
        self.assertVectorAlmostEquals(n_loc.position, (1, 2, 3), 5)
>       self.assertVectorAlmostEquals(n_loc.orientation, (180, -35, -127), 5)

test_direct_api.py:1595: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
test_direct_api.py:110: in assertVectorAlmostEquals
    self.assertAlmostEqual(first.X, second_vector.X, places, msg=msg)
E   AssertionError: -180.0 != 180.0 within 5 places (360.0 difference)
gumyr commented 7 months ago

I don't have an Apple Silicon Mac so I'm unable to work on this issue. Hopefully someone with the appropriate H/W will be able to help.

jdegenstein commented 7 months ago

The difference is 360 degrees, so maybe the solution is not to fix e.g. def test_make_brake_formed(self): or the underlying cause but instead to have a version of assertVectorAlmostEquals that takes this into account e.g. assertOrientationVectorAlmostEquals

MatthiasJ1 commented 7 months ago

Or migrate from euler angles to quaternions

MatthiasJ1 commented 7 months ago

I don't have an Apple Silicon Mac so I'm unable to work on this issue. Hopefully someone with the appropriate H/W will be able to help.

Just FYI, in case you ever want to test on M1 yourself, you can get an M1 VPS here for only 0.11/hr

jdegenstein commented 7 months ago

@MatthiasJ1 I created a (rough) prototype of orientation vector comparison that internally converts both inputs to quaternions, and then converts back into Euler angles and then compares those. It passes tests on all x86 platforms. Could you test it, available here: https://github.com/jdegenstein/build123d/blob/dev/tests/test_direct_api.py#L114

I did not point all the tests at the new comparison method, just the one that failed above (test_neg)

EDIT: I pointed all applicable tests at the new method and they all passed.

jdegenstein commented 7 months ago

@MatthiasJ1 tested orientation vector comparison via quaternions and it is still failing on M1 (this previously passed on all other platforms) https://github.com/jdegenstein/build123d/actions/runs/7645068147/job/20830789244?pr=4

     def test_neg(self):
         loc = Location((1, 2, 3), (0, 35, 127))
         n_loc = -loc
         self.assertVectorAlmostEquals(n_loc.position, (1, 2, 3), 5)
 >       self.assertOrientationVectorAlmostEquals(n_loc.orientation, (180, -35, -127), 5)

 tests/test_direct_api.py:1630: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 tests/test_direct_api.py:146: in assertOrientationVectorAlmostEquals
     self.assertAlmostEqual(first_vector.X, second_vector.X, places, msg=msg)
 E   AssertionError: -53.24031235481778 != 53.24031235481776 within 5 places (106.48062470963555 difference)