More rigid radial cutoff parameters in EGNN. #85
Closed
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 fell in the trap of specifying
radial_cutoff
andedges
inconsistently. Since the default value ofedges
isfully_connected
, specifying onlyradial_cutoff
does nothing and a fully connected calculation takes place.To avoid giving the user the ability to shoot themselves in the foot (as I did), here I implement a consistency check on these two inputs.
I also corrected a "bug" in the test suite: the EGNN model should not be equivariant to the octahedral point group for arbitrary basis vectors; it should only be equivariant for a cubic unit cell. It is weird that the tests passed with a fully connected graph: I suppose that the lattice vectors don't matter in that case...