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

Implementation ambiguity #4

Open
pinkfloyd06 opened this issue Mar 7, 2018 · 3 comments
Open

Implementation ambiguity #4

pinkfloyd06 opened this issue Mar 7, 2018 · 3 comments

Comments

@pinkfloyd06
Copy link

Hello @xbresson ,

Let me first thank for this work.

l went through your implementation. Hence , l have some question :

1- You set coarsening_levels = 4, however only L[0] and L[2] are used

x = self.graph_conv_cheby(x, self.cl1, L[0], lmax[0], self.CL1_F, self.CL1_K) and
x = self.graph_conv_cheby(x, self.cl2, L[2], lmax[2], self.CL2_F, self.CL2_K)
What about L[1],L[3] and L[4] ?

2- Why lmax[0] and lmax[2] are recalculated in graph_conv_cheby . Even if lmax is a parameter input of this function.

3- Do you mind explaining why your rescale Laplacian eigenvalues to [-1,1] ?
rescale_L(L, lmax)

4- concat(x,x_) is not ued at all in graph_conv_cheby() :

        def concat(x, x_):
            x_ = x_.unsqueeze(0)  # 1 x V x Fin*B
            return torch.cat((x, x_), 0)  # K x V x Fin*B

5- What if K=1 in graph_conv_cheby() ?
l noticed that at least K should equal 2.
How can l use that just for K=1 ?

Thank you for your consideration

@TheShadow29
Copy link

Hi @pinkfloyd06. I am not the author but I have read the code and I hope that I could answer some of those questions. Do note I too might have made some mistakes, feel free to correct me.

  1. The author uses graph_max_pool(x,4). In doing so the number of nodes reduces by a factor of 4 and therefore L[1] is not used.

  2. I think it doesn't matter. Either implementation would give the same result. You can compute lmax earlier and pass it as a parameter or recompute it inside the forward function. I am not sure if one of the two is better than the other in speed.

  3. I am clueless. Normalized laplacian has eigen values in the range 0 to 2. I am not sure why the range has been made -1 to 1.

  4. Yes. concat function has not been used.

  5. I am not very sure on this, but I think the line 232 should be if K >= 1 which should take care of the case.

@pinkfloyd06
Copy link
Author

@TheShadow29 Thank you for your answer :

  1. I do agree with you. But what l don't understand is the fact of computing L[1] L[3] and L[4] but never been used !!
    From my understanding l think for the second maxpooling layer we use L[1] rather than L[2] and if we have a third maxpolling layer it will be with L[2] , forth maxpooling L[3] and so on. Please correct me if l'm wrong

@TheShadow29
Copy link

@pinkfloyd06
As I earlier mentioned, the author used graph_max_pool(x, 4). So the number of nodes are divided by a factor of 4. In the computation of the laplacian matrices, the number of nodes decrease by a factor of 2. So L[1] refers to the laplacian of the graph which would be resulted when graph_max_pool(x, 2) would be used. But since we directed divide it by a factor of 4, we don't need L[1]. Instead we need L[2] which corresponds to the laplacian formed by the graph when it is divided by a factor of 4 (twice divided by factor of 2). Hope this helps.

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