fix: return empty array if molecule too big

cheminfo · Jul 29, 2024 · 89c9198 · 89c9198
1 parent f28d941
commit 89c9198
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 12 deletions.
diff --git a/src/topic/TopicMolecule.ts b/src/topic/TopicMolecule.ts
@@ -288,18 +288,17 @@ export class TopicMolecule {
   /**
    * This is related to the current moleculeWithH. The order is NOT canonized
    */
-  get diaIDs(): string[] | undefined {
+  get diaIDs(): string[] {
     if (this.cache.diaIDs) return this.cache.diaIDs;
+    const diaIDs = [];
     if (this.moleculeWithH.getAllAtoms() > this.options.maxNbAtoms) {
       this.options.logger.warn(
         `too many atoms to evaluate heterotopicity: ${this.moleculeWithH.getAllAtoms()} > ${this.options.maxNbAtoms}`,
       );
-      this.cache.diaIDs = undefined;
-      return undefined;
-    }
-    const diaIDs = [];
-    for (let i = 0; i < this.moleculeWithH.getAllAtoms(); i++) {
-      diaIDs.push(this.canonizedDiaIDs[this.finalRanks[i]]);
+    } else {
+      for (let i = 0; i < this.moleculeWithH.getAllAtoms(); i++) {
+        diaIDs.push(this.canonizedDiaIDs[this.finalRanks[i]]);
+      }
     }
     this.cache.diaIDs = diaIDs;
     return diaIDs;
@@ -334,7 +333,10 @@ export class TopicMolecule {
 
   private get canonizedDiaIDs() {
     if (this.cache.canonizedDiaIDs) return this.cache.canonizedDiaIDs;
-    this.cache.canonizedDiaIDs = getCanonizedDiaIDs(this);
+    this.cache.canonizedDiaIDs = getCanonizedDiaIDs(this, {
+      maxNbAtoms: this.options.maxNbAtoms,
+      logger: this.options.logger,
+    });
     return this.cache.canonizedDiaIDs;
   }
 
@@ -354,7 +356,9 @@ export class TopicMolecule {
   }
 
   get diaIDsAndInfo(): DiaIDAndInfo[] {
-    if (this.cache.diaIDsAndInfo) return this.cache.diaIDsAndInfo;
+    if (this.cache.diaIDsAndInfo) {
+      return this.cache.diaIDsAndInfo;
+    }
     this.cache.diaIDsAndInfo = getDiaIDsAndInfo(this, this.canonizedDiaIDs);
     return this.cache.diaIDsAndInfo;
   }

diff --git a/src/topic/__tests__/TopicMolecule.test.js b/src/topic/__tests__/TopicMolecule.test.js
@@ -15,7 +15,7 @@ describe('TopicMolecule', () => {
     const molecule = Molecule.fromSmiles('C1CCCC1'.repeat(20));
     const topicMolecule = new TopicMolecule(molecule, { logger });
 
-    expect(topicMolecule.diaIDs).toBeUndefined();
+    expect(topicMolecule.diaIDs).toStrictEqual([]);
     expect(topicMolecule.toMolfileWithH().split('\n')).toHaveLength(549);
     expect(logger.getLogs()).toHaveLength(2);
 
@@ -236,6 +236,8 @@ describe('TopicMolecule', () => {
     toggleHydrogens(molecule, 0);
     toggleHydrogens(molecule, 1);
     toggleHydrogens(molecule, 0);
+
+
     let advancedMolecule2 = topicMolecule.fromMolecule(molecule);
     let atoms = getAtomsAndDiaInfo(advancedMolecule2);
     expect(atoms).toHaveLength(6);

diff --git a/src/topic/getCanonizedDiaIDs.ts b/src/topic/getCanonizedDiaIDs.ts
@@ -1,11 +1,28 @@
+import { Logger } from 'cheminfo-types';
+
 import { makeRacemic } from '../util/makeRacemic.js';
 import { tagAtom } from '../util/tagAtom';
 
 import { TopicMolecule } from './TopicMolecule.js';
 
-export function getCanonizedDiaIDs(diaMol: TopicMolecule) {
-  const heterotopicSymmetryRanks = diaMol.heterotopicSymmetryRanks;
+export interface GetCanonizedDiaIDsOptions {
+  maxNbAtoms: number;
+  logger: Omit<Logger, 'child' | 'fatal'>;
+}
+
+export function getCanonizedDiaIDs(
+  diaMol: TopicMolecule,
+  options: GetCanonizedDiaIDsOptions,
+) {
+  const { logger, maxNbAtoms } = options;
   const moleculeWithH = diaMol.moleculeWithH;
+  if (moleculeWithH.getAllAtoms() > maxNbAtoms) {
+    logger.warn(
+      `too many atoms to evaluate heterotopic chiral bonds: ${moleculeWithH.getAllAtoms()} > ${maxNbAtoms}`,
+    );
+    return [];
+  }
+  const heterotopicSymmetryRanks = diaMol.heterotopicSymmetryRanks;
   const finalRanks = diaMol.finalRanks;
   const canonizedDiaIDs = new Array(moleculeWithH.getAllAtoms());
   moleculeWithH.ensureHelperArrays(