CFConv layer for SchNet #18
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is ready to review. I posted benchmark results at #14 (comment). |
raimis
reviewed
Nov 20, 2020
| * @param w2 an array of shape [width][width] containing the weights of the second dense layer | ||
| * @param b2 an array of length [width] containing the biases of the second dense layer | ||
| */ | ||
| CFConv(int numAtoms, int width, int numGaussians, float cutoff, bool periodic, float gaussianWidth) : |
There was a problem hiding this comment.
w1, b1, etc. is described above, but they are missing here.
There was a problem hiding this comment.
Good catch, I should remove those. The subclass constructors take those arguments, but they don't pass them on to the parent class.
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've written the CPU implementation so far. The calculation matches the behavior of SchNetPack, which is different from what's described in the paper. It's less flexible than SchNetPack though. For example, it always uses a cutoff, and always uses a cosine cutoff function, where SchNetPack lets you choose an arbitrary cutoff function.
The way I've implemented the derivatives is less than ideal. The standard method for doing backpropagation assumes you've saved intermediate quantities during the forward pass. But I don't want to save any per-interaction quantities, so instead I have to repeat nearly all the calculations from the forward pass in the backward pass. This slows it down a bit, but I think it will still end up being much faster on the GPU than if we save and reload per-interaction data. Once the CUDA implementation is written we can investigate that assumption and see if it's true.