Skip to content

Commit 5157b0b

Browse files
committed
Performance optimization, SMSD 3.2.0, expanded test suite
- Fix busy-wait spin loop in GraphMatcher (use awaitTermination) - Optimize MCSThread: HashMap for atom counting, multiset intersection - Remove synchronized methods from BondChangeCalculator, Holder, CalculationProcess - Remove redundant Collections.synchronized* wrappers - Remove unused DEBUG flags and redundant if(DEBUG) guards around LOGGER - Upgrade SMSD 3.1.0 -> 3.2.0 - Add 9 new biochemistry tests (glycosidic hydrolysis, phosphodiester, beta-lactam, retro-aldol, thioester, epoxide, Michael addition, deamination, transamination) - Re-enable R08765 and Rhea10074 tests - All 40 tests pass, net -88 lines Author: Syed Asad Rahman <asad.rahman@bioinceptionlabs.com>
1 parent c4b9a7a commit 5157b0b

23 files changed

Lines changed: 491 additions & 579 deletions

pom-local.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
<dependency>
4545
<groupId>com.bioinceptionlabs</groupId>
4646
<artifactId>smsd</artifactId>
47-
<version>3.1.0</version>
47+
<version>3.2.0</version>
4848
</dependency>
4949

5050
<dependency>

pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@
113113
<dependency>
114114
<groupId>com.bioinceptionlabs</groupId>
115115
<artifactId>smsd</artifactId>
116-
<version>3.1.0</version>
116+
<version>3.2.0</version>
117117
</dependency>
118118

119119
<!-- https://mvnrepository.com/artifact/commons-cli/commons-cli -->

src/main/java/uk/ac/ebi/aamtool/rgroup/ECRgroupFrequency.java

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,7 @@ public ECRgroupFrequency(String[] args) {
107107
}
108108
}
109109

110-
if (DEBUG) {
111-
LOGGER.debug("Number of EC parsed " + reactionMap.size());
112-
}
110+
LOGGER.debug("Number of EC parsed " + reactionMap.size());
113111

114112
int ec1Counter = 0;
115113
int ec2Counter = 0;
@@ -153,38 +151,32 @@ public ECRgroupFrequency(String[] args) {
153151
&& reactionMap.get(ec).getCommonDifferenceFP().isEmpty()
154152
&& !reactionMap.get(ec).isRGroup()) {
155153
no_common_fragment_in_non_r_group++;
156-
if (DEBUG) {
157-
LOGGER.debug("Processing EC: " + ec
158-
+ ", R-found: " + reactionMap.get(ec).isRGroup()
159-
+ ", common fragment: " + reactionMap.get(ec).getCommonCommonFP()
160-
+ ", difference fragment: " + reactionMap.get(ec).getCommonDifferenceFP()
161-
+ ", reaction count: " + reactionMap.get(ec).getReactionCount());
162-
}
154+
LOGGER.debug("Processing EC: " + ec
155+
+ ", R-found: " + reactionMap.get(ec).isRGroup()
156+
+ ", common fragment: " + reactionMap.get(ec).getCommonCommonFP()
157+
+ ", difference fragment: " + reactionMap.get(ec).getCommonDifferenceFP()
158+
+ ", reaction count: " + reactionMap.get(ec).getReactionCount());
163159
}
164160
if (reactionMap.get(ec).getCommonCommonFP().isEmpty()
165161
&& reactionMap.get(ec).getCommonDifferenceFP().isEmpty()
166162
&& reactionMap.get(ec).isRGroup()) {
167163
no_common_fragment_in_r_group++;
168-
if (DEBUG) {
169164

170-
LOGGER.debug("Processing EC: " + ec
171-
+ ", R-found: " + reactionMap.get(ec).isRGroup()
172-
+ ", common fragment: " + reactionMap.get(ec).getCommonCommonFP()
173-
+ ", difference fragment: " + reactionMap.get(ec).getCommonDifferenceFP()
174-
+ ", reaction count: " + reactionMap.get(ec).getReactionCount());
175-
}
165+
LOGGER.debug("Processing EC: " + ec
166+
+ ", R-found: " + reactionMap.get(ec).isRGroup()
167+
+ ", common fragment: " + reactionMap.get(ec).getCommonCommonFP()
168+
+ ", difference fragment: " + reactionMap.get(ec).getCommonDifferenceFP()
169+
+ ", reaction count: " + reactionMap.get(ec).getReactionCount());
176170
}
177171

178172
if (reactionMap.get(ec).getCommonCommonFP().isEmpty()
179173
&& reactionMap.get(ec).getCommonDifferenceFP().isEmpty()) {
180174
no_common_fragment_in_either++;
181-
if (DEBUG) {
182-
LOGGER.debug("Processing EC: " + ec
183-
+ ", R-found: " + reactionMap.get(ec).isRGroup()
184-
+ ", common fragment: " + reactionMap.get(ec).getCommonCommonFP()
185-
+ ", difference fragment: " + reactionMap.get(ec).getCommonDifferenceFP()
186-
+ ", reaction count: " + reactionMap.get(ec).getReactionCount());
187-
}
175+
LOGGER.debug("Processing EC: " + ec
176+
+ ", R-found: " + reactionMap.get(ec).isRGroup()
177+
+ ", common fragment: " + reactionMap.get(ec).getCommonCommonFP()
178+
+ ", difference fragment: " + reactionMap.get(ec).getCommonDifferenceFP()
179+
+ ", reaction count: " + reactionMap.get(ec).getReactionCount());
188180
}
189181

190182
String ec3 = reactionMap.get(ec).getEnzyme1Level() + "." + reactionMap.get(ec).getEnzyme2Level() + "." + reactionMap.get(ec).getEnzyme3Level();

src/main/java/uk/ac/ebi/aamtool/rgroup/MultiReactionContainer.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
*/
1919
package uk.ac.ebi.aamtool.rgroup;
2020

21-
import static uk.ac.ebi.aamtool.rgroup.ECRgroupFrequency.DEBUG;
2221
import java.util.HashSet;
2322
import java.util.List;
2423
import java.util.Set;
@@ -203,16 +202,12 @@ private void calculateCommonFingerprint(IReaction reaction) {
203202
Set<String> common = new HashSet<>(l);
204203
boolean intersection = common.retainAll(r);
205204

206-
if (DEBUG) {
207-
LOGGER.debug("intersection " + common);
208-
}
205+
LOGGER.debug("intersection " + common);
209206

210207
Set<String> difference = new TreeSet<>(l);
211208
difference.addAll(r);
212209
boolean removeAll = difference.removeAll(common);
213-
if (DEBUG) {
214-
LOGGER.debug("difference " + difference);
215-
}
210+
LOGGER.debug("difference " + difference);
216211
/*
217212
if no reaction is present then add all the patterns
218213
*/

src/main/java/uk/ac/ebi/reactionblast/mapping/CallableAtomMappingTool.java

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@
5959
public class CallableAtomMappingTool implements Serializable {
6060

6161
static final String NEW_LINE = getProperty("line.separator");
62-
private final static boolean DEBUG = false;
6362
private final static ILoggingTool LOGGER
6463
= createLoggingTool(CallableAtomMappingTool.class);
6564
private static final long serialVersionUID = 0x29e2adb1716b13eL;
@@ -126,10 +125,8 @@ private synchronized void generateAtomAtomMapping(
126125
*/
127126
LOGGER.info(NEW_LINE + "|++++++++++++++++++++++++++++|");
128127
LOGGER.info("b) Local Model: ");
129-
if (DEBUG) {
130-
LOGGER.debug(NEW_LINE + "-----------------------------------" + NEW_LINE);
131-
LOGGER.debug(NEW_LINE + "STEP b: Local Model Standardize Reactions" + NEW_LINE);
132-
}
128+
LOGGER.debug(NEW_LINE + "-----------------------------------" + NEW_LINE);
129+
LOGGER.debug(NEW_LINE + "STEP b: Local Model Standardize Reactions" + NEW_LINE);
133130
IReaction cleanedReaction1 = null;
134131
try {
135132
cleanedReaction1 = standardizer.standardize(reaction);
@@ -145,20 +142,16 @@ private synchronized void generateAtomAtomMapping(
145142
*/
146143
LOGGER.info(NEW_LINE + "|++++++++++++++++++++++++++++|");
147144
LOGGER.info("a) Global Model: ");
148-
if (DEBUG) {
149-
LOGGER.debug(NEW_LINE + "-----------------------------------" + NEW_LINE);
150-
LOGGER.debug(NEW_LINE + "STEP 1: Global Model Standardize Reactions" + NEW_LINE);
151-
}
145+
LOGGER.debug(NEW_LINE + "-----------------------------------" + NEW_LINE);
146+
LOGGER.debug(NEW_LINE + "STEP 1: Global Model Standardize Reactions" + NEW_LINE);
152147
IReaction cleanedReaction2 = null;
153148
try {
154149
cleanedReaction2 = standardizer.standardize(reaction);
155150
} catch (Exception e) {
156151
LOGGER.debug("ERROR: in AtomMappingTool: " + e.getMessage());
157152
LOGGER.error(e);
158153
}
159-
if (DEBUG) {
160-
LOGGER.debug(NEW_LINE + "STEP a: Calling Mapping Models" + NEW_LINE);
161-
}
154+
LOGGER.debug(NEW_LINE + "STEP a: Calling Mapping Models" + NEW_LINE);
162155
MappingThread maxThread = new MappingThread("IMappingAlgorithm.MAX", cleanedReaction2, MAX, removeHydrogen);
163156
cs.submit(maxThread);
164157
jobCounter++;
@@ -168,10 +161,8 @@ private synchronized void generateAtomAtomMapping(
168161
*/
169162
LOGGER.info(NEW_LINE + "|++++++++++++++++++++++++++++|");
170163
LOGGER.info("c) Mixture Model: ");
171-
if (DEBUG) {
172-
LOGGER.debug(NEW_LINE + "-----------------------------------" + NEW_LINE);
173-
LOGGER.debug(NEW_LINE + "STEP c: Mixture Model Standardize Reactions" + NEW_LINE);
174-
}
164+
LOGGER.debug(NEW_LINE + "-----------------------------------" + NEW_LINE);
165+
LOGGER.debug(NEW_LINE + "STEP c: Mixture Model Standardize Reactions" + NEW_LINE);
175166
IReaction cleanedReaction3 = null;
176167
try {
177168
cleanedReaction3 = standardizer.standardize(reaction);
@@ -188,10 +179,8 @@ private synchronized void generateAtomAtomMapping(
188179
*/
189180
LOGGER.info(NEW_LINE + "|++++++++++++++++++++++++++++|");
190181
LOGGER.info("d) Rings Model: ");
191-
if (DEBUG) {
192-
LOGGER.debug(NEW_LINE + "-----------------------------------" + NEW_LINE);
193-
LOGGER.debug(NEW_LINE + "STEP d: Rings Model Standardize Reactions" + NEW_LINE);
194-
}
182+
LOGGER.debug(NEW_LINE + "-----------------------------------" + NEW_LINE);
183+
LOGGER.debug(NEW_LINE + "STEP d: Rings Model Standardize Reactions" + NEW_LINE);
195184
IReaction cleanedReaction4 = null;
196185
try {
197186
cleanedReaction4 = standardizer.standardize(reaction);
@@ -218,19 +207,15 @@ private synchronized void generateAtomAtomMapping(
218207
*/
219208
while (!executor.isTerminated()) {
220209
}
221-
if (DEBUG) {
222-
LOGGER.debug("======DONE CallableAtomMappingTool=======");
223-
}
210+
LOGGER.debug("======DONE CallableAtomMappingTool=======");
224211
gc();
225212
} catch (InterruptedException | ExecutionException e) {
226213
LOGGER.debug("ERROR: in AtomMappingTool: " + e.getMessage());
227214
LOGGER.error(e);
228215
} finally {
229216
executor.shutdown();
230217
}
231-
if (DEBUG) {
232-
LOGGER.debug("!!!!Atom-Atom Mapping Done!!!!");
233-
}
218+
LOGGER.debug("!!!!Atom-Atom Mapping Done!!!!");
234219
/*
235220
* Mapping cache cleared
236221
*/

src/main/java/uk/ac/ebi/reactionblast/mapping/Reactor.java

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,8 @@ public class Reactor extends AbstractReactor implements Serializable {
149149

150150
this.substrateAtomCounter = 1;
151151
this.productAtomCounter = 1;
152-
if (DEBUG) {
153-
LOGGER.debug("|++++++++++++++++++++++++++++|");
154-
LOGGER.debug("|i. Reactor Initialized");
155-
}
152+
LOGGER.debug("|++++++++++++++++++++++++++++|");
153+
LOGGER.debug("|i. Reactor Initialized");
156154
cleanMapping(reaction);
157155
if (DEBUG) {
158156
LOGGER.debug("|++++++++++++++++++++++++++++|");
@@ -162,9 +160,7 @@ public class Reactor extends AbstractReactor implements Serializable {
162160
copyReferenceReaction(reaction);
163161
expandReaction();
164162
checkReactionBalance();
165-
if (DEBUG) {
166-
LOGGER.debug("|iii. Compute atom-atom Mappings");
167-
}
163+
LOGGER.debug("|iii. Compute atom-atom Mappings");
168164
calculateAtomAtomMapping();
169165
if (DEBUG) {
170166
printReaction(reactionWithUniqueSTOICHIOMETRY);
@@ -1140,9 +1136,7 @@ private synchronized IAtomContainer prepareMol(IAtomContainer cloneMolecule)
11401136
try {
11411137
//this helps to avoid concurrent modification error, reason unknown
11421138
String sm = unique().create(cloneMolecule, p);
1143-
if (DEBUG) {
1144-
LOGGER.debug("smiles " + sm);
1145-
}
1139+
LOGGER.debug("smiles " + sm);
11461140
} catch (CDKException e) {
11471141
LOGGER.error(SEVERE, null, e);
11481142
}
@@ -1193,9 +1187,7 @@ private synchronized IAtomContainer prepareMol(IAtomContainer cloneMolecule)
11931187
*/
11941188
private synchronized void permuteWithoutClone(int[] p, IAtomContainer atomContainer) {
11951189
int n = atomContainer.getAtomCount();
1196-
if (DEBUG) {
1197-
LOGGER.debug("permuting " + java.util.Arrays.toString(p));
1198-
}
1190+
LOGGER.debug("permuting " + java.util.Arrays.toString(p));
11991191
IAtom[] permutedAtoms = new IAtom[n];
12001192

12011193
for (int i = 0; i < n; i++) {

0 commit comments

Comments
 (0)