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

Smoothing Algorithms #321

Merged
merged 5 commits into from
Feb 17, 2019
Merged

Smoothing Algorithms #321

merged 5 commits into from
Feb 17, 2019

Conversation

bbarroqueiro
Copy link
Contributor

@bbarroqueiro bbarroqueiro commented Feb 15, 2019

Hello, i added smoothing.py which as four def's. The Laplacian calculation which is the most expensive step. Then three filters: laplacian, humphrey, taubin, which can be applied multiple times in different orders in an i
inexpensive way...

@coveralls
Copy link

coveralls commented Feb 15, 2019

Coverage Status

Coverage decreased (-0.3%) to 83.717% when pulling 25ec237 on bbarroqueiro:master into 4b486af on mikedh:master.

= 2 - Umbrella Weights
Output: Laplacian operator (scipy sparse matrix)
"""
vert_neigh=mesh.vertex_neighbors; vert=mesh.vertices; vert_ori=mesh.vertex_normals.copy(); n_vert=len(vert)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we split these out into separate lines without semicolons?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done... Thanks for the suggestion...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No prob man, thanks! I didn't say this before, but nice work! This will be very useful!!!

@mikedh
Copy link
Owner

mikedh commented Feb 15, 2019

Looks great, huge improvement thanks! I'll probably change up the formatting when I merge (remove semicolons like @Marviel suggested, remove trimesh import), though one thing before I merge: could you add a unit test file, tests/test_smoothing.py? I'd copy an existing one as a template and then add a test something like:

m = g.trimesh.creation.icosahedron()
s = g.trimesh.smoothing.laplacian(m)

assert m.is_volume
assert s.is_volume

# I don't know what these ratios are supposed to be but this should be repeatable
assert g.np.isclose(m.area / s.area, AREA_RATIO)
assert g.np.isclose(m.volume / s.volume, VOLUME_RATIO)

@bbarroqueiro
Copy link
Contributor Author

bbarroqueiro commented Feb 15, 2019

I can try... But it is not easy to state a test... The Laplacian filter is known to reduce volume and Humphry and Taubin tries to address the issue...

There always be some changes in volume and specially in area and it depends how severe the user sets the diffusion. If the diffusion is to severe large changes in shape can occur and the filter can be working just fine...

I can think about it, see if i can come up with some test... I'm open to suggestions...

@mikedh
Copy link
Owner

mikedh commented Feb 15, 2019

Hey, yeah no worries, the test mainly needs to be repeatable, it's not necessarily "exactly assess whether the solution is correct or not." The value is making sure that the smoothing routines are run in CI so we can make sure that given the same parameters changes elsewhere don't break the functionality.

Thanks!

@bbarroqueiro
Copy link
Contributor Author

Sorry, but i'm new to github an programming....

How can i test my "test_smoothing.py" code? Before committing it?

@mikedh
Copy link
Owner

mikedh commented Feb 15, 2019

No worries, you may be running in to an import paths issue: you can either put the directory with the trimesh repo into the PYTHONPATH env variable, here's mine:

mikedh@cue:tests$ echo $PYTHONPATH
/media/psf/Dropbox/robotics/pycollada:/media/psf/Dropbox/robotics/pypocketing:/media/psf/Dropbox/robotics/pyactp:/home/mikedh/trimesh:

You can also do an editable pip install from inside the trimesh directory, which will do basically the same thing:
pip install -e .

I'd put the following in tests/test_smoothing.py and then run python test_smoothing.py.

try:
    from . import generic as g
except BaseException:
    import generic as g


class SmoothTest(g.unittest.TestCase):
    def test_smooth(self):
        """
        Load a collada scene with pycollada.
        """

        m = g.trimesh.creation.icosahedron()
	s = g.trimesh.smoothing.laplacian(m)

	assert m.is_volume
	assert s.is_volume

	# I don't know what these ratios are supposed to be but this should be repeatable
	assert g.np.isclose(m.area / s.area, AREA_RATIO)
	assert g.np.isclose(m.volume / s.volume, VOLUME_RATIO)

if __name__ == '__main__':
    g.trimesh.util.attach_to_log()
    g.unittest.main()

Or if you just have a code block that works as a script (with some assert's) just send me a copy and I can put it in the right place.

@bbarroqueiro
Copy link
Contributor Author

Hello, i added a test file hopefully it is properly done... But, i will have read a bit about how git works so far a just my script in vs code with anaconda and that's it...

About the algorithms, these are classic well know algorithms much more advanced algorithms are available.

Now i'm working in a new weighting function... At a given vertices and its neighbour (that forms an edge i need to know the area of the two triangles that forms the edge...
My current solution quite slow, i'm checking all nearby_faces of all vertices and then i intersect the nearby faces of vertice and nearby_face of its neighbour. Thus, i get the wanted two elements and then their area... Any better ideas?

Regards
Bruno

@mikedh mikedh changed the base branch from master to merge/filter February 17, 2019 18:02
@mikedh
Copy link
Owner

mikedh commented Feb 17, 2019

Looks terrific, thanks! I'll merge to a branch for testing and profiling and then get it into master ASAP.

This is a huge improvement, thanks again for taking the time to PR!

@mikedh mikedh merged commit 159454d into mikedh:merge/filter Feb 17, 2019
mikedh added a commit that referenced this pull request Feb 17, 2019
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