-
Notifications
You must be signed in to change notification settings - Fork 44
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
Unable to instantiate R3IcoConv
#52
Comments
Hi @kalekundert Thanks a lot for spotting this issue! I introduced a small bug recently which indeed broke the steerable bases used to construct R3IcoConv. This is fixed now! Regarding the question:
I would generally recommend using R3Conv always. The R3IcoConv is using an unstable steerable basis (it often doesn't pass the equivariance tests) and it was implemented to demonstrate the benefits of the harmonic bases used in R3Conv (see Figure 5 in our paper). I am realising now that this is not emphasised in the documentation. I'm sorry for the confusion, I'll add a warning about this!
Yes, that's correct, as long as you use an output regular representation.
This library implements general steerable CNNs, of which group convolution is just a special case given by the choice of the regular representation. For these reasons, we still need the irreps of the Icosahedral group to implement a GCNN equivariant to this group.
This should have been fixed thanks to your pull-request ;) Hope this helps and sorry for the delay in my answer! |
I'm running into an error when trying to use the
R3IcoConv
module. Here's some code that generates the error:And here's the error itself:
I tried a few different kernel sizes and both the
'ico'
and'dodeca'
samples, but neither had any effect. I'm not sure if this is a bug, or if I'm doing something wrong.On a related note, I'm not totally sure I understand when to use
R3IcoConv
vsR3Conv
. Below are some bullet points outlining my thoughts on this. Can you take a look and let me know if I'm on the right track or not?R3Conv
, the kernel parameters are used to make linear combinations of spherical harmonics, so you can think of the parameters as Fourier coefficients.R3IcoConv
, the kernel parameters are associated with vertices of a icosahedron/dodecahedron/icosidodecahedron, so they live entirely in the spatial domain and aren't Fourier coefficients.R3IcoConv
should work better. If you want smooth filters,R3Conv
is fine.R3IcoConv
is more efficient, since it's specialized to the specific case of icosahedral symmetry, but I have no idea if this is true.This leads to a few follow-up questions:
R3Conv
instead ofR3IcoConv
with theicoOnR3
gspace?icoOnR3
gspace. Are these irreps actually used byR3IcoConv
? I know that in the 2D case, you can do group convolutions that are C(n)-equivariant with standard, unconstrained kernels by "lifting" the input to a higher dimension. CanR3IcoConv
be thought of as a 3D version of this? If so, I'd expect that it wouldn't be necessary to calculate irreps. I suspect that this analogy is just wrong, but I'm not sure.Thanks for taking the time to help me understand this. I've watched Erik Bekkers' lecture series on equivariant learning, so I feel like I have a some grasp on the fundamentals, but this is all still quite new to me.
The text was updated successfully, but these errors were encountered: