Mesh.units are no longer unit-vectors

[bwoodsend]

With numpy-stl<=2.10.1, Mesh.units are normalised to have a magnitude of 1. In version 2.11.2 however this appears to no longer be the case.

Here's a generic code that demonstrates the issue:

import numpy as np
from stl import mesh

# Define the 8 vertices of the cube
vertices = np.array([\
    [-1, -1, -1],
    [+1, -1, -1],
    [+1, +1, -1],
    [-1, +1, -1],
    [-1, -1, +1],
    [+1, -1, +1],
    [+1, +1, +1],
    [-1, +1, +1]])
# Define the 12 triangles composing the cube
faces = np.array([\
    [0,3,1],
    [1,3,2],
    [0,4,7],
    [0,7,3],
    [4,5,6],
    [4,6,7],
    [5,1,2],
    [5,2,6],
    [2,3,6],
    [3,7,6],
    [0,1,5],
    [0,5,4]])

This used to give me:

``` Should all have magnitude of 1: [[ 0. 0. -1.] [ 0. 0. -1.] [-1. 0. 0.] [-1. 0. 0.] [ 0. 0. 1.] [ 0. 0. 1.] [ 1. 0. 0.] [ 1. -0. 0.] [ 0. 1. -0.] [ 0. 1. 0.] [ 0. -1. 0.] [ 0. -1. 0.]] Should all equal 1: [1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1.] ```

But now I get:

``` Should all have magnitude of 1: [[ 0. 0. -0.25] [ 0. 0. -0.25] [-0.25 0. 0. ] [-0.25 0. 0. ] [ 0. 0. 0.25] [ 0. 0. 0.25] [ 0.25 0. 0. ] [ 0.25 -0. 0. ] [ 0. 0.25 -0. ] [ 0. 0.25 0. ] [ 0. -0.25 0. ] [ 0. -0.25 0. ]] Should all equal 1: [0.0625 0.0625 0.0625 0.0625 0.0625 0.0625 0.0625 0.0625 0.0625 0.0625 0.0625 0.0625] ```

I've bumped into this issue on Python 3.8 and 3.7. Happy to track down and submit a PR if you're super busy...

[wolph]

I thought this issue was fixed here: https://github.com/WoLpH/numpy-stl/issues/108 But I guess it's still broken :/

If you're able to create a PR it would be greatly appreciated. I still have a lot of issues to get to and have no time to take care of them right now

[bwoodsend]

Ahh I see the problem.

Originally mesh.normals was the raw un-normalised normals directly from the cross product which, due to the way the cross product works, had magnitudes of mesh.areas / 2. And mesh.units normalises them to unit-vectors by dividing by their magnitude mesh.units = mesh.normals / mesh.areas / 2.

Now however, #108 changed this so that mesh.normals are already normalised (using np.linalg.norm). mesh.units hasn't been updated to reflect this however as it still trying to normalise using mesh.units = mesh.normals / mesh.areas / 2 which assumes mesh.normals have magnitudes of mesh.areas / 2. Hence mesh.units now have magnitudes of 2 / mesh.areas.

Possible solutions I see 2 possible options. @WoLpH Your call!

1. Revert #108 - anyone who needs normalised normals can already use mesh.units which will work on any version of numpy-stl expect this broken one.
2. Make Mesh.units an alias of Mesh.normals which will ensure that Mesh.units are actually unit-vectors (as implied by the name) but will change the behaviour of Mesh.normals in any future versions of numpy-stl (including the current one).

If I get any say in the matter - I vote the 1st. Normalising mesh.normals is redundant, changes existing behaviour and having normals who's magnitudes are proportional to their areas is surprisingly handy for doing weighted statistics that are independent of the mesh resolution.

[wolph]

Oops... very sorry about the slow response. I think solution 1 sounds the best. It makes sense to me at least.

[bwoodsend]

Actually looking at the format spec for STL, it should be normalised normals that go into the file. But that would mean:

• changing Mesh.dtype to dtype([('units', '<f4', (3,)), ('vectors', '<f4', (3, 3)), ('attr', '<u2', (1,))]),
• then making Mesh.units equivalent to Mesh.data["units"]

Which sounds like a terrible idea considering almost every software I know treats the normals in the file as junk, preferring to calculate them from vectors.

I'll fire over a PR to revert #108.

[wolph]

Which sounds like a terrible idea considering almost every software I know treats the normals in the file as junk, preferring to calculate them from vectors.

Yeah... I tend to agree. That would definitely cause issues.

Thank you for all the help!

[wolph]

I've merged the changes and added an extra method in case people really want the unit normals: get_unit_normals()

That should make everyone happy :)