Skip to content

Commit 6c48357

Browse files
committed
Fix three correctness and memory issues found in code review
MappingDiagnostics: change snapshot() to use REACTIONS.remove() instead of get() — entries for completed reactions now released immediately after the snapshot is consumed, preventing unbounded map growth in batch runs. StandardizeReaction: preserve stoichiometric coefficients when rebuilding the filtered reaction. addReactant/addProduct without a coefficient silently resets it to 1.0; downstream ReactionContainer reads the coefficient for atom-count weighting, so this was corrupting balance checks for any reaction with non-unit coefficients. RDT API: getTotalBondChanges/getFormedCleavedBonds/getOrderChangedBonds now sum the encoded weights in each "PATTERN:N" feature string rather than counting unique pattern types. Two formed C-O bonds reported as "C-O:2" should count as 2, not 1. ReactionResult.similarity() likewise keeps full "PATTERN:N" strings so reactions differing only in stoichiometry have distinct fingerprints.
1 parent cb14f0b commit 6c48357

4 files changed

Lines changed: 30 additions & 19 deletions

File tree

src/main/java/com/bioinceptionlabs/reactionblast/api/RDT.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ private static ReactionResult extractResult(ReactionMechanismTool rmt, String in
138138
return new ReactionResult(
139139
inputSmiles,
140140
mappedSmiles,
141-
formedCleaved.size(),
142-
orderChanges.size(),
143-
stereoChanges.size(),
141+
weightSum(formedCleaved),
142+
weightSum(orderChanges),
143+
weightSum(stereoChanges),
144144
formedCleaved,
145145
orderChanges,
146146
stereoChanges,
@@ -157,4 +157,19 @@ private static List<String> extractFeatures(IPatternFingerprinter fp) {
157157
}
158158
return features;
159159
}
160+
161+
/** Sum the integer weights encoded in "PATTERN:N" feature strings. */
162+
private static int weightSum(List<String> features) {
163+
int total = 0;
164+
for (String f : features) {
165+
int colon = f.lastIndexOf(':');
166+
if (colon > 0) {
167+
try { total += Integer.parseInt(f.substring(colon + 1)); }
168+
catch (NumberFormatException e) { total += 1; }
169+
} else {
170+
total += 1;
171+
}
172+
}
173+
return total;
174+
}
160175
}

src/main/java/com/bioinceptionlabs/reactionblast/api/ReactionResult.java

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -169,25 +169,19 @@ public double similarity(ReactionResult other) {
169169

170170
/**
171171
* Get all fingerprint features as a combined set (for similarity).
172-
* Uses pattern names only (strips weight suffix) for robust matching.
173-
* E.g., "C-O:1" → "C-O", so reactions with same bond types match.
172+
* Keeps the full "PATTERN:WEIGHT" strings so that "C-O:2" and "C-O:1"
173+
* are treated as distinct, giving accurate Tanimoto scores for reactions
174+
* that differ only in stoichiometry.
174175
*/
175176
private Set<String> getAllFingerprints() {
176177
Set<String> all = new HashSet<>();
177-
addPatterns(all, formedCleavedBonds);
178-
addPatterns(all, orderChangedBonds);
179-
addPatterns(all, stereoChangedBonds);
180-
addPatterns(all, reactionCentreFingerprint);
178+
all.addAll(formedCleavedBonds);
179+
all.addAll(orderChangedBonds);
180+
all.addAll(stereoChangedBonds);
181+
all.addAll(reactionCentreFingerprint);
181182
return all;
182183
}
183184

184-
private static void addPatterns(Set<String> set, List<String> features) {
185-
for (String f : features) {
186-
int colon = f.lastIndexOf(':');
187-
set.add(colon > 0 ? f.substring(0, colon) : f);
188-
}
189-
}
190-
191185
/**
192186
* Tanimoto coefficient: |A ∩ B| / |A ∪ B|
193187
*/

src/main/java/com/bioinceptionlabs/reactionblast/mapping/MappingDiagnostics.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public static void recordEvaluationPhase(String reactionId, long elapsedMillis)
100100
}
101101

102102
public static ReactionSnapshot snapshot(String reactionId) {
103-
ReactionStats stats = REACTIONS.get(reactionId);
103+
ReactionStats stats = REACTIONS.remove(reactionId);
104104
return stats == null
105105
? new ReactionSnapshot(reactionId, 0L, 0L, Collections.emptyList())
106106
: stats.snapshot(reactionId);

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,10 +343,12 @@ public IReaction filterReagents(IReaction reaction) {
343343
filtered.setID(reaction.getID());
344344
filtered.setDirection(reaction.getDirection());
345345
for (IAtomContainer r : keptReactants) {
346-
filtered.addReactant(r);
346+
Double coeff = reaction.getReactantCoefficient(r);
347+
filtered.addReactant(r, coeff != null ? coeff : 1.0);
347348
}
348349
for (IAtomContainer p : products.atomContainers()) {
349-
filtered.addProduct(p);
350+
Double coeff = reaction.getProductCoefficient(p);
351+
filtered.addProduct(p, coeff != null ? coeff : 1.0);
350352
}
351353
for (IAtomContainer agent : reagents) {
352354
filtered.addAgent(agent);

0 commit comments

Comments
 (0)