Skip to content

Commit 6c60fca

Browse files
asadasad
committed
Speed up test workflow and harden mapping canonicalisation
1 parent c5557c7 commit 6c60fca

27 files changed

Lines changed: 308 additions & 99 deletions

‎README.md‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,20 @@ Installation
5050

5151
```
5252
use pom.xml and mvn commands to build your project
53-
1) mvn clean compile (compile only)
54-
2) mvn clean test (compile and run tests)
55-
3) mvn clean install -DskipTests=true (install, skip tests)
56-
4) mvn clean install (install with tests)
57-
5) mvn -P local clean install -DskipTests=true (fat jar, skip tests)
58-
6) mvn -P local clean install (fat jar with tests)
53+
1) mvn clean compile (compile only, no tests)
54+
2) mvn clean test (fast regression suite only)
55+
3) mvn -P full-tests clean test (extended regression suites)
56+
4) mvn -P benchmarks clean test (benchmark suites only)
57+
5) mvn clean install -DskipTests=true (install, skip tests)
58+
6) mvn -P local clean install -DskipTests=true (fat jar, skip tests)
59+
7) mvn -P local,full-tests clean install (fat jar with extended tests)
5960
```
6061

62+
Default test runs are intentionally lightweight. They skip the exhaustive
63+
dataset sweeps and benchmark suites. Test image generation is also disabled by
64+
default; re-enable it with `-Drdt.generate.test.images=true` if you need PNG
65+
artifacts during test runs.
66+
6167
Simple Java API (Recommended)
6268
==============================
6369

‎pom.xml‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
<maven.compiler.target>21</maven.compiler.target>
1414
<cdk.version>2.12</cdk.version>
1515
<mainClass>com.bioinceptionlabs.aamtool.ReactionDecoder</mainClass>
16+
<surefire.groups></surefire.groups>
17+
<surefire.excludedGroups>com.bioinceptionlabs.testgroups.FullRegression,com.bioinceptionlabs.testgroups.Benchmark</surefire.excludedGroups>
18+
<rdt.generate.test.images>false</rdt.generate.test.images>
1619
</properties>
1720

1821
<scm>
@@ -44,10 +47,6 @@
4447
<name>BioInception</name>
4548
<url>https://bioinceptionlabs.com</url>
4649
</organization>
47-
<prerequisites>
48-
<maven>3.8</maven>
49-
</prerequisites>
50-
5150
<developers>
5251
<developer>
5352
<name>Syed Asad Rahman</name>
@@ -176,12 +175,6 @@
176175
<artifactId>cdk-legacy</artifactId>
177176
<version>${cdk.version}</version>
178177
</dependency>
179-
<dependency>
180-
<groupId>org.openscience.cdk</groupId>
181-
<artifactId>cdk-inchi</artifactId>
182-
<version>${cdk.version}</version>
183-
</dependency>
184-
185178
<dependency>
186179
<groupId>com.bioinceptionlabs</groupId>
187180
<artifactId>smsd</artifactId>
@@ -195,12 +188,6 @@
195188
<version>1.11.0</version>
196189
</dependency>
197190

198-
<dependency>
199-
<groupId>commons-io</groupId>
200-
<artifactId>commons-io</artifactId>
201-
<version>2.21.0</version>
202-
</dependency>
203-
204191
<dependency>
205192
<groupId>com.google.guava</groupId>
206193
<artifactId>guava</artifactId>
@@ -218,11 +205,9 @@
218205
<artifactId>maven-compiler-plugin</artifactId>
219206
<version>3.14.1</version>
220207
<configuration>
221-
<source>${jdk.version}</source>
222-
<target>${jdk.version}</target>
208+
<release>${jdk.version}</release>
223209
<showDeprecation>false</showDeprecation>
224210
<compilerArgs>
225-
<arg>-Xlint:-deprecation</arg>
226211
<arg>-Xlint:-unchecked</arg>
227212
</compilerArgs>
228213
</configuration>
@@ -254,6 +239,25 @@
254239
</plugins>
255240
</build>
256241
</profile>
242+
<profile>
243+
<id>full-tests</id>
244+
<properties>
245+
<surefire.excludedGroups>com.bioinceptionlabs.testgroups.Benchmark</surefire.excludedGroups>
246+
</properties>
247+
</profile>
248+
<profile>
249+
<id>benchmarks</id>
250+
<properties>
251+
<surefire.groups>com.bioinceptionlabs.testgroups.Benchmark</surefire.groups>
252+
<surefire.excludedGroups></surefire.excludedGroups>
253+
</properties>
254+
</profile>
255+
<profile>
256+
<id>all-tests</id>
257+
<properties>
258+
<surefire.excludedGroups></surefire.excludedGroups>
259+
</properties>
260+
</profile>
257261
<profile>
258262
<id>disable-java8-doclint</id>
259263
<activation>
@@ -272,9 +276,9 @@
272276
<artifactId>maven-compiler-plugin</artifactId>
273277
<version>3.14.1</version>
274278
<configuration>
279+
<release>${jdk.version}</release>
275280
<showDeprecation>false</showDeprecation>
276281
<compilerArgs>
277-
<arg>-Xlint:-deprecation</arg>
278282
<arg>-Xlint:-unchecked</arg>
279283
</compilerArgs>
280284
</configuration>
@@ -285,6 +289,12 @@
285289
<version>3.5.5</version>
286290
<configuration>
287291
<argLine>--enable-native-access=ALL-UNNAMED</argLine>
292+
<groups>${surefire.groups}</groups>
293+
<excludedGroups>${surefire.excludedGroups}</excludedGroups>
294+
<systemPropertyVariables>
295+
<java.awt.headless>true</java.awt.headless>
296+
<rdt.generate.test.images>${rdt.generate.test.images}</rdt.generate.test.images>
297+
</systemPropertyVariables>
288298
</configuration>
289299
</plugin>
290300
</plugins>

‎src/main/java/com/bioinceptionlabs/aamtool/Annotator.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ public class Annotator extends Helper {
118118
* @return
119119
* @throws Exception
120120
*/
121+
@SuppressWarnings("deprecation")
121122
protected static ReactionMechanismTool getReactionMechanismTool(IReaction cdkReaction,
122123
boolean reMap, boolean complexMappingFlag, boolean accept_no_change) throws Exception {
123124
ReactionMechanismTool rmt;

‎src/main/java/com/bioinceptionlabs/aamtool/Helper.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ protected static void displayBlankLines(final int numberBlankLines, final Output
115115
protected static void printHelp(final OutputStream out, final Options options) {
116116
final String commandLineSyntax = "java -jar ReactionDecoder.jar";
117117
try (PrintWriter writer = new PrintWriter(out)) {
118-
final HelpFormatter formatter = new HelpFormatter();
118+
final HelpFormatter formatter = HelpFormatter.builder().get();
119119
displayBlankLines(2, out);
120120
formatter.printHelp(writer, 80, commandLineSyntax, "HELP",
121121
options, 5, 3, "End of Helper Help", true);
@@ -129,7 +129,7 @@ protected static void printHelp(final Map<String, Options> optionsMap, final int
129129
final int spacesBeforeOptionDescription, final boolean displayUsage, final OutputStream out) {
130130
final String commandLineSyntax = "java -jar ReactionDecoder.jar";
131131
try (PrintWriter writer = new PrintWriter(out)) {
132-
final HelpFormatter helpFormatter = new HelpFormatter();
132+
final HelpFormatter helpFormatter = HelpFormatter.builder().get();
133133
optionsMap.keySet().stream().map((headerString) -> {
134134
helpFormatter.printHelp(
135135
writer,

‎src/main/java/com/bioinceptionlabs/reactionblast/cdk/CDKToolkit.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public CDKToolkit() {
7878
this.canonicalSmilesGen = new SmilesGenerator(SmiFlavor.Canonical);
7979
this.mappedSmilesGen = new SmilesGenerator(
8080
SmiFlavor.Stereo | SmiFlavor.AtomAtomMap);
81-
this.aromaticity = new Aromaticity(ElectronDonation.daylight(),
81+
this.aromaticity = new Aromaticity(ElectronDonation.piBonds(),
8282
Cycles.or(Cycles.all(), Cycles.or(Cycles.relevant(), Cycles.essential())));
8383
}
8484

‎src/main/java/com/bioinceptionlabs/reactionblast/mapping/Reactor.java‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,7 @@ private void checkReactionBalance() throws IOException {
353353
}
354354
}
355355

356+
@SuppressWarnings("deprecation")
356357
private void calculateAtomAtomMapping() throws IOException, Exception {
357358

358359
try {
@@ -790,6 +791,7 @@ private List<IAtom> markHAroundCoreAtoms(String id, IAtomContainerSet molSet) {
790791
* @param molSet
791792
* @return
792793
*/
794+
@SuppressWarnings("deprecation")
793795
private List<IAtom> collectUnMappedSingleHAtoms(IAtomContainerSet molSet) {
794796

795797
List<IAtom> list = new ArrayList<>();
@@ -836,6 +838,7 @@ private List<IAtom> collectUnMappedHAtoms(IAtomContainerSet molSet) {
836838
* @param counter
837839
* @return updated Counter
838840
*/
841+
@SuppressWarnings("deprecation")
839842
private int markUnMappedHAtoms(IReaction mappedReaction, int counter) {
840843

841844
int localCounter = counter;
@@ -1185,13 +1188,14 @@ private IAtomContainer prepareMol(IAtomContainer cloneMolecule)
11851188
*/
11861189
private void permuteWithoutClone(int[] p, IAtomContainer atomContainer) {
11871190
int n = atomContainer.getAtomCount();
1191+
int[] permutation = normalizePermutation(p, n);
11881192
LOGGER.debug("permuting " + java.util.Arrays.toString(p));
11891193
IAtom[] permutedAtoms = new IAtom[n];
11901194

11911195
for (int i = 0; i < n; i++) {
11921196
IAtom atom = atomContainer.getAtom(i);
1193-
permutedAtoms[p[i]] = atom;
1194-
atom.setProperty("label", p[i]);
1197+
permutedAtoms[permutation[i]] = atom;
1198+
atom.setProperty("label", permutation[i]);
11951199
}
11961200
atomContainer.setAtoms(permutedAtoms);
11971201

@@ -1222,6 +1226,29 @@ private void permuteWithoutClone(int[] p, IAtomContainer atomContainer) {
12221226
atomContainer.setBonds(bonds);
12231227
}
12241228

1229+
private int[] normalizePermutation(int[] permutation, int size) {
1230+
if (permutation == null || permutation.length != size) {
1231+
return identityPermutation(size);
1232+
}
1233+
1234+
boolean[] seen = new boolean[size];
1235+
for (int value : permutation) {
1236+
if (value < 0 || value >= size || seen[value]) {
1237+
return identityPermutation(size);
1238+
}
1239+
seen[value] = true;
1240+
}
1241+
return permutation;
1242+
}
1243+
1244+
private int[] identityPermutation(int size) {
1245+
int[] identity = new int[size];
1246+
for (int i = 0; i < size; i++) {
1247+
identity[i] = i;
1248+
}
1249+
return identity;
1250+
}
1251+
12251252
/**
12261253
* Old Atom Rank in the reactant mapped to new Rank
12271254
*
@@ -1705,6 +1732,7 @@ public static class MappingHandler extends BasicDebugger {
17051732
*
17061733
* @param MappedReaction
17071734
*/
1735+
@SuppressWarnings("deprecation")
17081736
public static void cleanMapping(IReaction MappedReaction) {
17091737
int count = MappedReaction.getMappingCount();
17101738
for (int i = count - 1; i >= 0; i--) {
@@ -1741,6 +1769,7 @@ public static void cleanMapping(IReaction MappedReaction) {
17411769
* @param counter
17421770
* @return
17431771
*/
1772+
@SuppressWarnings("deprecation")
17441773
protected static int setMappingFlags(IReaction expLabReaction, IReaction MappedReaction, int counter) {
17451774
IAtomContainerSet expEductSet = expLabReaction.getReactants();
17461775
IAtomContainerSet expProductSet = expLabReaction.getProducts();
@@ -1820,6 +1849,7 @@ protected static int setMappingFlags(IReaction expLabReaction, IReaction MappedR
18201849
* @param counter
18211850
* @return
18221851
*/
1852+
@SuppressWarnings("deprecation")
18231853
protected static int setMappingFlags(IReaction MappedReaction, IReaction ReactionWithUniqueSTOICHIOMETRY, IReaction coreMappedReaction, int counter) {
18241854
IAtomContainerSet expEductSet = ReactionWithUniqueSTOICHIOMETRY.getReactants();
18251855
IAtomContainerSet expProductSet = ReactionWithUniqueSTOICHIOMETRY.getProducts();

‎src/main/java/com/bioinceptionlabs/reactionblast/mapping/algorithm/CalculationProcess.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,14 +312,14 @@ private boolean findAndChipBond(IAtomContainer container, IAtomContainer referen
312312
&& bond.getAtom(1).getSymbol().equalsIgnoreCase("C"))
313313
|| (bond.getAtom(0).getSymbol().equalsIgnoreCase("C")
314314
&& bond.getAtom(1).getSymbol().equalsIgnoreCase("O"))) {
315-
if (!bond.getAtom(0).getFlag(ISAROMATIC)
316-
&& !bond.getAtom(1).getFlag(ISAROMATIC)) {
315+
if (!bond.getAtom(0).isAromatic()
316+
&& !bond.getAtom(1).isAromatic()) {
317317
if (referenceContainer.contains(bond)) {
318318
IAtom atom = bond.getAtom(0).getSymbol().equalsIgnoreCase("C") ? bond.getAtom(0) : bond.getAtom(1);
319319
List<IBond> neighbourhoodBonds = referenceContainer.getConnectedBondsList(atom);
320320
flag = false;
321321
for (IBond neighbourhoodBond : neighbourhoodBonds) {
322-
if (neighbourhoodBond.contains(atom) && !neighbourhoodBond.getFlag(ISINRING)) {
322+
if (neighbourhoodBond.contains(atom) && !neighbourhoodBond.isInRing()) {
323323
if ((neighbourhoodBond.getAtom(0).getSymbol().equalsIgnoreCase("O")
324324
&& neighbourhoodBond.getAtom(1).getSymbol().equalsIgnoreCase("C"))
325325
|| (neighbourhoodBond.getAtom(0).getSymbol().equalsIgnoreCase("C")

‎src/main/java/com/bioinceptionlabs/reactionblast/mechanism/BEMatrix.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
* @author Syed Asad Rahman<asad.rahman@bioinceptionlabs.com>
4747
* @author Lorenzo Baldacci {lorenzo@ebi.ac.uk|lbaldacc@csr.unibo.it}
4848
*/
49+
@SuppressWarnings("deprecation")
4950
public class BEMatrix extends EBIMatrix implements Serializable {
5051

5152
private static final long serialVersionUID = -1420740601548197863L;

‎src/main/java/com/bioinceptionlabs/reactionblast/mechanism/BondChangeAnnotator.java‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ public void writeRMatrix(File outputFile) {
243243
*
244244
* @throws Exception
245245
*/
246+
@SuppressWarnings("deprecation")
246247
protected void markBondChanges() throws Exception {
247248
ensureReactionMatrices();
248249

@@ -694,13 +695,13 @@ private IBond getBondOfProductsByRMatrix(IAtom atom1, IAtom atom2) {
694695
*/
695696
public int isKekuleEffect(IBond affectedBondReactants, IBond affectedBondProducts) {
696697
if (affectedBondReactants != null && affectedBondProducts != null) {
697-
if (affectedBondReactants.getFlag(ISINRING)
698-
== affectedBondProducts.getFlag(ISINRING)) {
698+
if (affectedBondReactants.isInRing()
699+
== affectedBondProducts.isInRing()) {
699700

700-
if ((!affectedBondReactants.getFlag(ISAROMATIC)
701-
&& affectedBondProducts.getFlag(ISAROMATIC))
702-
|| (affectedBondReactants.getFlag(ISAROMATIC)
703-
&& !affectedBondProducts.getFlag(ISAROMATIC))) {
701+
if ((!affectedBondReactants.isAromatic()
702+
&& affectedBondProducts.isAromatic())
703+
|| (affectedBondReactants.isAromatic()
704+
&& !affectedBondProducts.isAromatic())) {
704705
IRingSet smallestRingSetR = getSmallestRingSet(affectedBondReactants, queryRingSet);
705706
IRingSet smallestRingSetP = getSmallestRingSet(affectedBondProducts, targetRingSet);
706707

@@ -747,13 +748,13 @@ public int isKekuleEffect(IBond affectedBondReactants, IBond affectedBondProduct
747748
*/
748749
public int isAlternateKekuleChange(IBond affectedBondReactants, IBond affectedBondProducts) {
749750
if (affectedBondReactants != null && affectedBondProducts != null) {
750-
if (affectedBondReactants.getFlag(ISINRING)
751-
== affectedBondProducts.getFlag(ISINRING)) {
751+
if (affectedBondReactants.isInRing()
752+
== affectedBondProducts.isInRing()) {
752753

753-
if ((!affectedBondReactants.getFlag(ISAROMATIC)
754-
&& affectedBondProducts.getFlag(ISAROMATIC))
755-
|| (affectedBondReactants.getFlag(ISAROMATIC)
756-
&& !affectedBondProducts.getFlag(ISAROMATIC))) {
754+
if ((!affectedBondReactants.isAromatic()
755+
&& affectedBondProducts.isAromatic())
756+
|| (affectedBondReactants.isAromatic()
757+
&& !affectedBondProducts.isAromatic())) {
757758
IRingSet smallestRingSetR = getSmallestRingSet(affectedBondReactants, queryRingSet);
758759
IRingSet smallestRingSetP = getSmallestRingSet(affectedBondProducts, targetRingSet);
759760
int countR = getNeighbourBondOrderCountFromRing(affectedBondReactants, smallestRingSetR);

‎src/main/java/com/bioinceptionlabs/reactionblast/mechanism/BondChangeCalculator.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,7 @@ public Map<IAtom, String> getStereoCenterAtomsProduct() {
589589
*
590590
* @return (removed the unchanged H atoms)
591591
*/
592+
@SuppressWarnings("deprecation")
592593
public IReaction getReactionWithCompressUnChangedHydrogens() {
593594
IReaction compressedReaction = null;
594595

@@ -665,7 +666,7 @@ public IReaction getReactionWithCompressUnChangedHydrogens() {
665666
}
666667

667668
private static void KekulizeReaction(IReaction r) throws CDKException {
668-
ElectronDonation model = ElectronDonation.daylight();
669+
ElectronDonation model = ElectronDonation.piBonds();
669670
// CycleFinder cycles = Cycles.or(Cycles.all(), Cycles.all(6));
670671
// Aromaticity aromaticity = new Aromaticity(model, cycles);
671672

@@ -686,6 +687,7 @@ private static void KekulizeReaction(IReaction r) throws CDKException {
686687
*
687688
* @param reaction
688689
*/
690+
@SuppressWarnings("deprecation")
689691
private void cleanMapping(IReaction reaction) {
690692

691693
int count = reaction.getMappingCount();

0 commit comments

Comments
 (0)