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

Question about partition along the radial(r) dimension #4

Open
hi-zhengcheng opened this issue Oct 23, 2019 · 1 comment
Open

Question about partition along the radial(r) dimension #4

hi-zhengcheng opened this issue Oct 23, 2019 · 1 comment

Comments

@hi-zhengcheng
Copy link

Thanks for your nice paper and code, it helps me a lot.

Question 1

I’m trying to figure out how to partition along radial dimension. In the paper, section “3.1 Spherical Convolutions” has one sentence said:

We allow the partitions along the radial (r) dimension to be non-uniform because the cubic volume growth for large radius values can be undesirable

In the source code, it looks like that the partitions along the radial dimension are uniform:
https://github.com/hlei-ziyan/SPH3D-GCN/blob/27a0629b908e736d28b69723f333af29f63bea5c/tf_ops/buildkernel/tf_buildkernel_gpu.cu#L68

Could you share some experience or detailed method, about non-uniform partitions along the radial dimension?


Question 2

In source code, the 3D distance info (variable dist) is used for partitions along the radial dimension, and it is computed in:
https://github.com/hlei-ziyan/SPH3D-GCN/blob/27a0629b908e736d28b69723f333af29f63bea5c/tf_ops/nnquery/tf_nnquery_gpu.cu#L47

https://github.com/hlei-ziyan/SPH3D-GCN/blob/27a0629b908e736d28b69723f333af29f63bea5c/tf_ops/nnquery/tf_nnquery_gpu.cu#L54

…
dist3D = sqrtf(dist3D); //sqrt

if (dist3D<radius && fabs(dist3D-radius)>1e-6) // find a neighbor in range
{
    if (s<nnSample)
    {
        nnIndex[i*M*nnSample+j*nnSample+s] = k;
        nnDist[i*M*nnSample+j*nnSample+s] = sqrt(dist3D);
    }
    …
}

dist3D is the result of sqrtf first. When it is saved to nnDist, another sqrt is called. Why use sqrt two times, Is there any trick about this?

@EnyaHermite
Copy link
Owner

Actually, we tried only uniform radial division up till now. As to the two sqrt function, it is a bug. I do find it as well, but occupied by other things, I haven’t correct it yet. It doesn’t influence the experiment at all. You can remove the second sqrt in your code.

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

No branches or pull requests

2 participants