Skip to content

Commit 789142d

Browse files
committed
Algorithm optimizations: fix critical bounds bug, O(1) lookups, faster init
- CRITICAL: Fix EBIMatrix bounds check (i <= rows → i < rows) preventing silent out-of-bounds access on matrix edge indices - EBIMatrix: Use Arrays.fill() for initMatrix (native vs manual loop) - GraphMatcher: Replace O(n) linear atom-by-ID scan with O(1) HashMap lookup — builds ID→Atom maps once, then O(1) per mapping pair - GraphMatcher: Scale thread pool cap from 4 to 8 for multi-core systems - Holder: Combine setFingerprint() + setMolMapping() into single loop, pre-cache fingerprints to avoid redundant map lookups All 40 tests pass. Author: Syed Asad Rahman <asad.rahman@bioinceptionlabs.com>
1 parent 8bee8f7 commit 789142d

3 files changed

Lines changed: 57 additions & 54 deletions

File tree

src/main/java/com/bioinceptionlabs/reactionblast/mapping/algorithm/Holder.java

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,8 @@ public Holder(
9999
this.structureInformation = reactionContainer;
100100
this.bestMatchContainer = bestMatchContainer;
101101
this.hydFPFree = hydFPFree;
102-
LOGGER.debug("setFingerprint");
103-
setFingerprint();
104-
LOGGER.debug("setMolMapping");
105-
setMolMapping();
102+
LOGGER.debug("setFingerprintAndMolMapping");
103+
setFingerprintAndMolMapping();
106104
}
107105

108106
/**
@@ -171,32 +169,40 @@ private void initialize() {
171169
carbonOverlapMatrix.initMatrix(0.0);
172170
}
173171

174-
private void setFingerprint() {
175-
for (int i = 0; i < eductCounter.size(); i++) {
176-
for (int j = 0; j < productCounter.size(); j++) {
177-
try {
178-
String eductName = eductCounter.get(i).trim();
179-
String productName = productCounter.get(j).trim();
180-
BitSet hydrogenEductFP = hydFPFree.getFingerPrint(eductName);
181-
BitSet hydrogenProductFP = hydFPFree.getFingerPrint(productName);
182-
float hydrogenSimVal = getTanimotoSimilarity(hydrogenEductFP, hydrogenProductFP);
183-
LOGGER.debug("FP " + hydrogenSimVal);
184-
fpSimMatrixWithoutHydrogen.setValue(i, j, hydrogenSimVal);
185-
} catch (Exception ex) {
186-
LOGGER.error(SEVERE, null, ex);
187-
}
172+
private void setFingerprintAndMolMapping() {
173+
// Pre-cache trimmed names and fingerprints to avoid redundant lookups
174+
int eSize = eductCounter.size();
175+
int pSize = productCounter.size();
176+
String[] eNames = new String[eSize];
177+
BitSet[] eFPs = new BitSet[eSize];
178+
for (int i = 0; i < eSize; i++) {
179+
eNames[i] = eductCounter.get(i).trim();
180+
try {
181+
eFPs[i] = hydFPFree.getFingerPrint(eNames[i]);
182+
} catch (Exception ex) {
183+
LOGGER.error(SEVERE, null, ex);
188184
}
189185
}
190-
}
191-
192-
private void setMolMapping() {
193-
for (int i = 0; i < eductCounter.size(); i++) {
194-
for (int j = 0; j < productCounter.size(); j++) {
186+
String[] pNames = new String[pSize];
187+
BitSet[] pFPs = new BitSet[pSize];
188+
for (int j = 0; j < pSize; j++) {
189+
pNames[j] = productCounter.get(j).trim();
190+
try {
191+
pFPs[j] = hydFPFree.getFingerPrint(pNames[j]);
192+
} catch (Exception ex) {
193+
LOGGER.error(SEVERE, null, ex);
194+
}
195+
}
196+
// Single combined loop for fingerprint similarity + mol mapping
197+
for (int i = 0; i < eSize; i++) {
198+
for (int j = 0; j < pSize; j++) {
195199
try {
196-
String eductName = eductCounter.get(i).trim();
197-
String productName = productCounter.get(j).trim();
198-
MolMapping m = new MolMapping(eductName, productName, i, j);
199-
getMappingMolPair().add(m);
200+
if (eFPs[i] != null && pFPs[j] != null) {
201+
float hydrogenSimVal = getTanimotoSimilarity(eFPs[i], pFPs[j]);
202+
LOGGER.debug("FP " + hydrogenSimVal);
203+
fpSimMatrixWithoutHydrogen.setValue(i, j, hydrogenSimVal);
204+
}
205+
getMappingMolPair().add(new MolMapping(eNames[i], pNames[j], i, j));
200206
} catch (Exception ex) {
201207
LOGGER.error(SEVERE, null, ex);
202208
}

src/main/java/com/bioinceptionlabs/reactionblast/mapping/graph/GraphMatcher.java

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ public synchronized static Collection<MCSSolution> matcher(Holder mh) throws Exc
154154
threadsAvailable = jobMap.size();
155155
}
156156

157-
if (threadsAvailable > 4) {
158-
threadsAvailable = 4;
157+
if (threadsAvailable > 8) {
158+
threadsAvailable = 8;
159159
}
160160

161161
LOGGER.debug(threadsAvailable + " threads requested for MCS in " + mh.getTheory());
@@ -385,13 +385,24 @@ static MCSSolution replicateMappingOnContainers(Holder mh, Combination solution,
385385

386386
AtomAtomMapping atomAtomMapping = mcs.getAtomAtomMapping();
387387
AtomAtomMapping atomAtomMappingNew = new AtomAtomMapping(q, t);
388-
atomAtomMapping.getMappingsByAtoms().keySet().stream().forEach((a) -> {
389-
IAtom atomByID1 = getAtomByID(q, a);
390-
IAtom b = atomAtomMapping.getMappingsByAtoms().get(a);
391-
IAtom atomByID2 = getAtomByID(t, b);
392-
// if (DEBUG) {
393-
// out.println("atomByID1 " + atomByID1.getID() + " atomByID2 " + atomByID2.getID());
394-
// }
388+
389+
// Build ID→Atom maps for O(1) lookup instead of O(n) linear scan
390+
Map<String, IAtom> qIdMap = new java.util.HashMap<>();
391+
for (IAtom a : q.atoms()) {
392+
if (a.getID() != null) {
393+
qIdMap.put(a.getID(), a);
394+
}
395+
}
396+
Map<String, IAtom> tIdMap = new java.util.HashMap<>();
397+
for (IAtom a : t.atoms()) {
398+
if (a.getID() != null) {
399+
tIdMap.put(a.getID(), a);
400+
}
401+
}
402+
403+
atomAtomMapping.getMappingsByAtoms().forEach((a, b) -> {
404+
IAtom atomByID1 = a.getID() != null ? qIdMap.get(a.getID()) : null;
405+
IAtom atomByID2 = b.getID() != null ? tIdMap.get(b.getID()) : null;
395406
if (atomByID1 != null && atomByID2 != null) {
396407
atomAtomMappingNew.put(atomByID1, atomByID2);
397408
} else {
@@ -410,15 +421,4 @@ static MCSSolution replicateMappingOnContainers(Holder mh, Combination solution,
410421
return null;
411422
}
412423

413-
private static IAtom getAtomByID(IAtomContainer ac, IAtom atom) {
414-
if (atom.getID() == null) {
415-
return null;
416-
}
417-
for (IAtom a : ac.atoms()) {
418-
if (atom.getID().equals(a.getID())) {
419-
return a;
420-
}
421-
}
422-
return null;
423-
}
424424
}

src/main/java/com/bioinceptionlabs/reactionblast/tools/EBIMatrix.java

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -410,9 +410,7 @@ public EBIMatrix(double vals[], int m) {
410410
*/
411411
public synchronized void initMatrix(double v) {
412412
for (int i = 0; i < rows; i++) {
413-
for (int j = 0; j < columns; j++) {
414-
matrix[i][j] = v;
415-
}
413+
java.util.Arrays.fill(matrix[i], v);
416414
}
417415
}
418416

@@ -427,11 +425,10 @@ public synchronized void initMatrix(double v) {
427425
public synchronized double getValue(int i, int j) {
428426

429427
double val = -1.0d;
430-
if (i <= rows && j <= columns) {
428+
if (i >= 0 && i < rows && j >= 0 && j < columns) {
431429
val = matrix[i][j];
432430
} else {
433-
434-
LOGGER.debug("Error: Array of out bound");
431+
LOGGER.debug("Error: Array out of bounds [" + i + "," + j + "] for [" + rows + "," + columns + "]");
435432
}
436433
return val;
437434
}
@@ -564,7 +561,7 @@ public synchronized boolean setValue(int row, int col, double value) {
564561
double val = value;
565562
boolean flag = false;
566563

567-
if (row <= rows && col <= columns) {
564+
if (row >= 0 && row < rows && col >= 0 && col < columns) {
568565
flag = true;
569566
matrix[row][col] = val;
570567
} else {

0 commit comments

Comments
 (0)