5
\$\begingroup\$

I'm totally new in Python and I wrote some code. It is a simple algorithm to smooth objects. I need to find adjacent vertices in mesh and sum their coordinates and after that divide by a number of adjacent vertices. This algorithm is called Laplacian Smoothing.

Could anybody point out how to make it better?

n = mesh.VPos.shape[0]
final = []

for i in range(n):
    neighbors = mesh.vertices[i].getVertexNeighbors()
    indices = map(lambda x: x.ID, neighbors)
    z = len(indices)

    help = []
    for j in indices:
        help.append([mesh.VPos[j]])
    final += [sum(x)/z for x in zip(*help)]

final = np.array(final)
mesh.VPos = final
\$\endgroup\$
3
\$\begingroup\$

The code is not that bad at all.

Here are a few notes:


  1. Use list comprehension – it's more readable than maps & lambdas
    indices = [x.ID for x in neighbors]
    

  1. indices isn't really necessary. Instead of

    indices = map(lambda x: x.ID, neighbors)
    z = len(indices)
    
    help = []
    for j in indices:
        help.append([mesh.VPos[j]])
    final += [sum(x)/z for x in zip(*help)]
    

    you can simply do this

    help = ([mesh.VPos[j.ID]] for j in neighbors)
    z = len(neighbors)
    final.extend(sum(x)/z for x in zip(*help))
    

    help is a generator expression here, since you don't really need a list. That will probably save some overhead of creating the list. You can also avoid creating a new list of sum(x)/z and directly append all values to final using the extend method.


  1. Instead of reassigning a numpy array to final, I'd just do
    mesh.VPos = np.array(final)
    

  1. I'm not completely sure, but I think the inner lists [mesh.VPos[j]] combined with zip(*help) will amount to the same as just
    for j in indices:
        help.append(mesh.VPos[j])
    final += [sum(help) / z]
    

  1. You can create a numpy.ndarray right at the beginning instead of using a Python list and then converting it.

So after the edits it'll look something like this

n = mesh.VPos.shape[0]
final = np.empty(n)  # change dtype if you need to

for i in range(n):
    neighbors = mesh.vertices[i].getVertexNeighbors()
    help = (mesh.VPos[j.ID] for j in neighbors)
    final[i] = sum(help) / len(neighbors)

mesh.VPos = final

Alternatively you can use numpy.mean to compute sum(help) / len(neighbors):

n = mesh.VPos.shape[0]
final = np.empty(n)  # change dtype if you need to

for i in range(n):
    neighbors = mesh.vertices[i].getVertexNeighbors()
    help = [mesh.VPos[j.ID] for j in neighbors]
    final[i] = np.mean(help)

mesh.VPos = final

Note that you cannot give a generator argument to numpy.mean, it has to be a list (or anything with __len__ method).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.