Skip to content

Commit

Permalink
feat: TopicMolecule failed if over 250 atoms but the main issues was …
Browse files Browse the repository at this point in the history
…DiaIDs

In the new version TopicMolecule accepts a logger in order to get feedback about degraded mode in which diaIDs are undefined.
  • Loading branch information
lpatiny committed Jul 29, 2024
1 parent 136f428 commit 9a9daf5
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 25 deletions.
38 changes: 31 additions & 7 deletions src/topic/TopicMolecule.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Logger } from 'cheminfo-types';
import type { Molecule } from 'openchemlib';

import { getHoseCodesForAtomsAsFragments } from '../hose/getHoseCodesForAtomsInternal.js';
Expand All @@ -19,22 +20,30 @@ interface ToMolfileOptions {
version?: 2 | 3;
}

interface TopicMoleculeOptions
extends HoseCodesOptions,
GetMoleculeWithHOptions {
interface TopicMoleculeOptions extends HoseCodesOptions {
/**
* The maximum path length to consider when calculating the paths between atoms
* @default 5
*/
maxPathLength?: number;
/**
* The maximum number of atoms to consider when dealing with diastereotopicity
*/
maxNbAtoms?: number;
/**
* The logger to use in order to retrieve some debug or warning information
* @default console
*/
logger?: Omit<Logger, 'child' | 'fatal'>;
}

type TopicMoleculeInternalOptions = Omit<
TopicMoleculeOptions,
'maxPathLength'
'maxPathLength' | 'maxNbAtoms' | 'logger'
> &
Required<Pick<TopicMoleculeOptions, 'maxPathLength'>>;
Required<
Pick<TopicMoleculeOptions, 'maxPathLength' | 'maxNbAtoms' | 'logger'>
>;

interface GetAtomPathOptions {
/*
Expand Down Expand Up @@ -91,7 +100,12 @@ export class TopicMolecule {

constructor(molecule: Molecule, options: TopicMoleculeOptions = {}) {
this.originalMolecule = molecule;
this.options = { maxPathLength: 5, ...options };
this.options = {
maxPathLength: 5,
maxNbAtoms: 250,
logger: console,
...options,
} as TopicMoleculeInternalOptions;
this.idCode = molecule.getIDCode();
this.molecule = this.originalMolecule.getCompactCopy();
this.molecule.ensureHelperArrays(
Expand Down Expand Up @@ -260,6 +274,7 @@ export class TopicMolecule {
if (this.cache.moleculeWithH) return this.cache.moleculeWithH;
this.cache.moleculeWithH = getMoleculeWithH(this.molecule, {
maxNbAtoms: this.options.maxNbAtoms,
logger: this.options.logger,
});
return this.cache.moleculeWithH;
}
Expand All @@ -273,8 +288,15 @@ export class TopicMolecule {
/**
* This is related to the current moleculeWithH. The order is NOT canonized
*/
get diaIDs(): string[] {
get diaIDs(): string[] | undefined {
if (this.cache.diaIDs) return this.cache.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]]);
Expand All @@ -289,6 +311,7 @@ export class TopicMolecule {
* @returns
*/
getDiaIDsObject() {
if (!this.diaIDs) return undefined;
return groupDiastereotopicAtomIDsAsObject(
this.diaIDs,
this.molecule,
Expand Down Expand Up @@ -381,6 +404,7 @@ export class TopicMolecule {
* @returns
*/
getGroupedDiastereotopicAtomIDs(options: GroupedDiaIDsOptions = {}) {
if (!this.diaIDs) return undefined;
return groupDiastereotopicAtomIDs(this.diaIDs, this.moleculeWithH, options);
}

Expand Down
22 changes: 14 additions & 8 deletions src/topic/__tests__/TopicMolecule.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { readFileSync } from 'fs';
import { join } from 'path';

import { FifoLogger } from 'fifo-logger';
import { Molecule } from 'openchemlib';
import { describe, it, expect } from 'vitest';

Expand All @@ -10,15 +11,20 @@ import { TopicMolecule } from '../TopicMolecule';
describe('TopicMolecule', () => {
it('Big molecule', () => {
//250 carbons
const molecule = Molecule.fromSmiles('C'.repeat(260));
const topicMolecule = new TopicMolecule(molecule);
expect(() => topicMolecule.toMolfileWithH()).toThrow(
'too many atoms to add hydrogens: 782 > 250',
);
const logger = new FifoLogger('test.log');
const molecule = Molecule.fromSmiles('C1CCCC1'.repeat(20));
const topicMolecule = new TopicMolecule(molecule, { logger });

const topicMolecule2 = new TopicMolecule(molecule, { maxNbAtoms: 1000 });
const molfile = topicMolecule2.toMolfileWithH();
expect(molfile.split('\n')).toHaveLength(1569);
expect(topicMolecule.diaIDs).toBeUndefined();
expect(topicMolecule.toMolfileWithH().split('\n')).toHaveLength(549);
expect(logger.getLogs()).toHaveLength(2);

const topicMolecule2 = new TopicMolecule(molecule, {
maxNbAtoms: 1000,
logger,
});
expect(topicMolecule2.diaIDs).toHaveLength(262);
expect(logger.getLogs()).toHaveLength(2);
});
it('ethanol', () => {
const molecule = Molecule.fromSmiles('CCCO');
Expand Down
19 changes: 9 additions & 10 deletions src/topic/getMoleculeWithH.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import { Logger } from 'cheminfo-types';
import type { Molecule } from 'openchemlib';

import { ensureHeterotopicChiralBonds } from '../diastereotopic/ensureHeterotopicChiralBonds.js';

export interface GetMoleculeWithHOptions {
/**
* @default 250
* Maximum number of atoms with hydrogens
*/
maxNbAtoms?: number;
maxNbAtoms: number;
logger: Omit<Logger, 'child' | 'fatal'>;
}

/**
Expand All @@ -17,16 +15,17 @@ export interface GetMoleculeWithHOptions {
*/
export function getMoleculeWithH(
molecule: Molecule,
options: GetMoleculeWithHOptions = {},
options: GetMoleculeWithHOptions,
) {
const { maxNbAtoms = 250 } = options;
const { logger, maxNbAtoms } = options;
const moleculeWithH = molecule.getCompactCopy();
moleculeWithH.addImplicitHydrogens();
if (moleculeWithH.getAllAtoms() > maxNbAtoms) {
throw new Error(
`too many atoms to add hydrogens: ${moleculeWithH.getAllAtoms()} > ${maxNbAtoms}`,
logger.warn(
`too many atoms to evaluate heterotopic chiral bonds: ${moleculeWithH.getAllAtoms()} > ${maxNbAtoms}`,
);
} else {
ensureHeterotopicChiralBonds(moleculeWithH);
}
ensureHeterotopicChiralBonds(moleculeWithH);
return moleculeWithH;
}

0 comments on commit 9a9daf5

Please sign in to comment.