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

Potential bug in OutputPPBlock of DimeNet++ #332

Closed
arunppsg opened this issue Apr 12, 2022 · 2 comments
Closed

Potential bug in OutputPPBlock of DimeNet++ #332

arunppsg opened this issue Apr 12, 2022 · 2 comments

Comments

@arunppsg
Copy link

In the following line in reset_parameters method of OutputPPBlock used in DimeNet++,

https://github.com/Open-Catalyst-Project/ocp/blob/b5a197fc3c79a9a5a787aabaa02979be53d296b7/ocpmodels/models/dimenet_plus_plus.py#L192

I think that initializing the weights of the linear layer to 0 will produce an output of 0. Probably, it should be
glorot_orthogonal(self.lin.weight, scale=2.0)

@mshuaibii
Copy link
Collaborator

mshuaibii commented Apr 12, 2022

Not quite. While it's true they are initialized to 0 and the very first forward pass outputs 0, a single backwards call will update these parameters to nonzero. It's always possible to explore better initialization schemes, but as is, this isn't an issue. The original DimeNet++ implementation offers the flexibility for both schemes, both working better for different targets

@arunppsg
Copy link
Author

Sorry, I overlooked it. Thanks!

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