Contact calculation performance tweaks - #1147
Conversation
aalhossary
left a comment
There was a problem hiding this comment.
I recommend 2 small updates and raised another concern that may need a substantial change.
This is Amr (Not the AI 😊).
| */ | ||
| private static final float DEFAULT_LOAD_FACTOR = 0.75f; | ||
|
|
||
| private HashMap<Pair<AtomIdentifier>, AtomContact> contacts; |
There was a problem hiding this comment.
AtomIdentifier contains only the Atom serial number and Chain ID. What about the Alternate Location? Isn't it important when we want to calculate contact matrix? What if an Atom is within the cutoff in an AltLoc but not in the other?
Taking into consideration the the Atom interface itself defines setAltLoc() and getAltLoc(), this may actually need a substantial change in the structure.
Or what do you think?
| } | ||
|
|
||
| public boolean hasContact(AtomIdentifier atomId1, AtomIdentifier atomId2) { | ||
| return contacts.containsKey(new Pair<AtomIdentifier>(atomId1,atomId2)); |
There was a problem hiding this comment.
Shouldn't the atom order be enforced here?
e.g.
if atom1 serial < atom2 serial then return hasContact(atomId2,atomId1)
| for (int b=0; b<numIindices; b++) { | ||
| int j = iIndices[b]; | ||
| if (j>i) { | ||
| double distance = iAtoms[i].distance(iAtoms[j]); | ||
| if (distance<cutoff) contacts.add(new Contact(i, j, distance)); | ||
| double distanceSq = atomI.distanceSquared(iAtoms[j]); | ||
| if (distanceSq<cutoffSq) contacts.add(new Contact(i, j, Math.sqrt(distanceSq))); | ||
| } | ||
| } |
There was a problem hiding this comment.
As long as you are comparing atoms of iAtoms to iAtoms (not to jAtoms), then you can safely make it:
for (int b = a+1; b<numIindices; b++) { // instead of int b = 0
and drop the check if (j>i) to be
for (int b = a + 1; b<numIindices; b++) {
int j = iIndices[b];
double distanceSq = atomI.distanceSquared(iAtoms[j]);
if (distanceSq<cutoffSq) contacts.add(new Contact(i, j, Math.sqrt(distanceSq)));
}
Covers what is merged since 7.2.6 and what is open and expected to land: the download and checksum work, the CATH and ECOD fixes, the contact and ASA performance tweaks, electron density, and the JUnit 5 migration. Five entries are for pull requests that are still open - biojava#1134, biojava#1147, biojava#1148, biojava#1150 and biojava#1151 - and should be checked against what actually merged before the release is tagged.
A few performance tweaks that increase contact calculation performance ~ 1.5x