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

Add a warning in remesch_botsch if all vertices are detected as boundary vertices #69

Merged
merged 2 commits into from
Feb 27, 2023

Conversation

EmJay276
Copy link
Contributor

If all vertices of a mesh are detected as boundary vertices, the remesh_botsch will return the input.
I've added a warning to make the user aware in this case (L65 - L67).

Additionally, I applied PEP8 style guide (whitespace) to the code.

@EmJay276
Copy link
Contributor Author

EmJay276 commented Feb 27, 2023

Due to my last post in #68 (comment) I am not sure if this should be merged.

I need to check first if the following example would also result in the same output as the input was

A ------ B
| \      | 
|   \    | 
|     \  | 
C------- D

@EmJay276
Copy link
Contributor Author

Minimal example for #69 (comment)

import gpytoolbox as gpy
import numpy as np

v = np.array(((0., 0., 0.),
              (10, 0., 0.),
              (0., 10, 0.),
              (10, 10, 0.)))

f = np.array(((0, 1, 3),
              (0, 2, 3)), dtype=np.int32)

u, g = gpy.remesh_botsch(v, f, h=1.)

print(f"Input: {v.shape = }, {f.shape = }")
print(f"Output: {u.shape = }, {g.shape = }")

Result

Input: v.shape = (4, 3), f.shape = (2, 3)
Output: u.shape = (4, 3), g.shape = (2, 3)

I would at least suggest to warn the user.
As user I would also expect the remesher should add vertices within the plane / at the edges.

@sgsellan
Copy link
Owner

This makes sense! Still, in L66, shouldn't we compare the length of V to the length of the unique entries of feature? Since feature is concatenating the user input and boundary vertices, its length could actually be larger than the size of V if, for example, I ran u, g = gpy.remesh_botsch(v, f, h=1.,feature=np.array([0],dtype=np.int32)) on your example above, which would not currently trigger the warning. Speaking off the cuff, free to tell me if I'm wrong!

@EmJay276
Copy link
Contributor Author

Indeed! Fixed with 7bdcd09

@sgsellan
Copy link
Owner

Awesome! I'll merge after checks.

Also, what is this PEP8 style guide? Should we apply it across the whole library?

@EmJay276
Copy link
Contributor Author

Also, what is this PEP8 style guide? Should we apply it across the whole library?

It's the Python Enhancement Proposals (PEP) for "Style Guide for Python Code"
https://peps.python.org/pep-0008/

PEP8 is mainly code formatting, which most Python IDEs automatically suggest. Including my IDE (PyCharm).

I'm my opinion it makes the code easier to read.

For example u = (0, 1 * 2) instead of u=(0,1*2)

@sgsellan sgsellan merged commit 8e11f80 into sgsellan:main Feb 27, 2023
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.

None yet

2 participants