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 farthest point sampling for points sampler, and modify the uniform sampler for the meshes #253

Merged
merged 5 commits into from
Aug 26, 2019

Conversation

hansen7
Copy link
Contributor

@hansen7 hansen7 commented Aug 14, 2019

Hi, Castro

I have added farthest point sampling into the points sampler, which is an algorithm applied to the cutting-edge research, see here for the relevant paper, and here for the description of the algorithm.

And I think it is probably better to sample u, v uniformly from [0, 1] and [0, u] respectively in the mesh sampler. Because to ensure the sampled points are within that face, it requires not only u + v <= 1 but also u >= 0 and v >= 0, but for the original procedure, which is sampled from Gaussian then truncation, it did not resolve the issues with negative sampling results. Plus it is more concise now.

I am very interested to make more commitments since it intersects with my research projects during PhD, I think next step is to develop the Poisson Disk Sampling as discussed here: #239, and is it okay for me to develop a new part for the normals/curvatures calculations(directly from the points) and with reconstruction on the doc as well.

Thanks.

@daavoo
Copy link
Owner

daavoo commented Aug 18, 2019

Hola @hansen7,

Thank you so much for the contribution! I'm leaving a few comments in the P.R. review.

Pd. I'm glad to hear that you are interested in making more commitments, don't hesitate to ask anything.

Copy link
Owner

@daavoo daavoo left a comment

Choose a reason for hiding this comment

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

Thank you again for the contribution, please don't hesitate in asking doubts about the comments and/or how to address them

pyntcloud/io/obj.py Show resolved Hide resolved
u[is_a_problem] = 1 - u[is_a_problem]
v[is_a_problem] = 1 - v[is_a_problem]
u = np.random.uniform(low=0., high=1., size=(self.n, 1))
v = np.random.uniform(low=0., high=u, size=(self.n, 1))
Copy link
Owner

@daavoo daavoo Aug 18, 2019

Choose a reason for hiding this comment

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

I really like this change!, it really simplify things. However I think that in order to maintain the expected¿? behaviour you should change:

v = np.random.uniform(low=0., high=1 - u, size=(n, 1))

Right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, yes, sorry for the typos, I was meant to write 1-u

pyntcloud/samplers/points.py Show resolved Hide resolved
@daavoo
Copy link
Owner

daavoo commented Aug 18, 2019

In addition to the above comments, please follow the steps described in:

https://pyntcloud.readthedocs.io/en/latest/samplers_dev.html#let-pyntcloud-know-about-your-sampler

In order to make your new sample available and easily accesible for everybody

Copy link
Owner

@daavoo daavoo left a comment

Choose a reason for hiding this comment

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

Thank you so much for taking the time to incluye the sugested changes!!

@daavoo daavoo merged commit 3142da3 into daavoo:master Aug 26, 2019
@hansen7
Copy link
Contributor Author

hansen7 commented Aug 26, 2019

haha no worries, is there other places for development, I am looking forward to more contributions

@hansen7 hansen7 deleted the hc_dev branch August 26, 2019 20:31
@hansen7 hansen7 restored the hc_dev branch August 26, 2019 20:33
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.

2 participants