Skip to content

Commit 9d0adcc

Browse files
refactor: code simplification pass on nvMolKit conformer worker
- Extract _rdkit_generate_chunk() to eliminate 3x duplicated blocks - Consolidate max_failures/max_iterations into single max_attempts - Extract _conf_has_finite_coords predicate in worker CLI - Compact setup script: str.removeprefix(), f-strings, shorter docstrings - Remove 8 obvious/redundant comments
1 parent 0147951 commit 9d0adcc

3 files changed

Lines changed: 68 additions & 119 deletions

File tree

matcha/utils/preprocessing.py

Lines changed: 25 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -383,11 +383,10 @@ def generate_multiple_conformers(orig_mol, num_conformers):
383383
mol = copy.deepcopy(orig_mol)
384384
ps = AllChem.ETKDGv3()
385385
failures, ids = 0, []
386-
max_failures = 3
387-
max_iterations = max_failures # Prevent infinite loops
386+
max_attempts = 3
388387

389388
iteration = 0
390-
while mol.GetNumConformers() < num_conformers and iteration < max_iterations:
389+
while mol.GetNumConformers() < num_conformers and iteration < max_attempts:
391390
current_count = mol.GetNumConformers()
392391
needed = num_conformers - current_count
393392

@@ -405,20 +404,18 @@ def generate_multiple_conformers(orig_mol, num_conformers):
405404

406405
ids = [id for id in ids if id != -1]
407406

408-
# Manually add each new conformer to the main molecule
409407
added_count = 0
410408
for conf_id in ids:
411-
conf = temp_mol.GetConformer(conf_id)
412-
mol.AddConformer(conf, assignId=True)
413-
added_count += 1
414-
409+
conf = temp_mol.GetConformer(conf_id)
410+
mol.AddConformer(conf, assignId=True)
411+
added_count += 1
412+
415413
new_count = mol.GetNumConformers()
416414

417415
if added_count == 0:
418-
# No new conformers were added
419-
logger.debug(f"No new conformers added. Retrying {iteration + 1}/{max_iterations}")
416+
logger.debug(f"No new conformers added. Retrying {iteration + 1}/{max_attempts}")
420417
failures += 1
421-
if failures >= max_failures:
418+
if failures >= max_attempts:
422419
break
423420
else:
424421
# Successfully added some conformers, reset failure counter
@@ -452,9 +449,9 @@ def generate_multiple_conformers(orig_mol, num_conformers):
452449
if mol.GetNumConformers() == 0:
453450
logger.warning("No conformers generated, using original molecule")
454451
return orig_mol
455-
else:
456-
logger.debug(f"Generated {mol.GetNumConformers()} conformers")
457-
return mol
452+
453+
logger.debug(f"Generated {mol.GetNumConformers()} conformers")
454+
return mol
458455

459456

460457
def generate_conformer_mols(orig_mol, num_conformers, backend: str | None = None):
@@ -501,7 +498,6 @@ def generate_conformer_mols_batch(
501498

502499
results = [None] * len(mols)
503500

504-
# Prepare molecules: keep originals intact for safety.
505501
prepared = []
506502
for mol in mols:
507503
init = copy.deepcopy(mol)
@@ -520,6 +516,17 @@ def generate_conformer_mols_batch(
520516
use_worker = backend == "worker" or (backend == "auto" and worker_cmd_configured)
521517
worker_disabled = False
522518

519+
def _rdkit_generate_chunk(chunk_mols):
520+
result = []
521+
for m in chunk_mols:
522+
m_with_confs = generate_multiple_conformers(m, int(confs_per_mol))
523+
m_with_confs = _remove_hs_safe(m_with_confs)
524+
result.append(_split_single_conformer_mols(m_with_confs, int(confs_per_mol)))
525+
if "rdkit" not in _CONFORMER_BACKEND_LOGGED:
526+
logger.info("Conformer backend: RDKit")
527+
_CONFORMER_BACKEND_LOGGED.add("rdkit")
528+
return result
529+
523530
for start in range(0, len(prepared), chunk_size):
524531
end = min(len(prepared), start + chunk_size)
525532
chunk = prepared[start:end]
@@ -538,29 +545,13 @@ def generate_conformer_mols_batch(
538545
logger.info("Conformer backend: worker")
539546
_CONFORMER_BACKEND_LOGGED.add("worker")
540547
else:
541-
chunk_results = []
542-
for mol in chunk:
543-
mol_with_confs = generate_multiple_conformers(mol, int(confs_per_mol))
544-
mol_with_confs = _remove_hs_safe(mol_with_confs)
545-
chunk_results.append(
546-
_split_single_conformer_mols(mol_with_confs, int(confs_per_mol))
547-
)
548-
if "rdkit" not in _CONFORMER_BACKEND_LOGGED:
549-
logger.info("Conformer backend: RDKit")
550-
_CONFORMER_BACKEND_LOGGED.add("rdkit")
548+
chunk_results = _rdkit_generate_chunk(chunk)
551549
except Exception as exc:
552550
if backend == "worker":
553551
raise RuntimeError(f"Worker conformer generation failed: {exc}") from exc
554552
worker_disabled = True
555553
logger.warning(f"Worker conformer backend failed, falling back to RDKit: {exc}")
556-
chunk_results = []
557-
for mol in chunk:
558-
mol_with_confs = generate_multiple_conformers(mol, int(confs_per_mol))
559-
mol_with_confs = _remove_hs_safe(mol_with_confs)
560-
chunk_results.append(_split_single_conformer_mols(mol_with_confs, int(confs_per_mol)))
561-
if "rdkit" not in _CONFORMER_BACKEND_LOGGED:
562-
logger.info("Conformer backend: RDKit")
563-
_CONFORMER_BACKEND_LOGGED.add("rdkit")
554+
chunk_results = _rdkit_generate_chunk(chunk)
564555

565556
for local_idx, conformer_mols in enumerate(chunk_results):
566557
processed = []
@@ -582,7 +573,6 @@ def generate_conformer_mols_batch(
582573
processed = _split_single_conformer_mols(fallback, 1)
583574
results[start + local_idx] = processed
584575

585-
# Satisfy type checker: results is fully populated.
586576
return [r if r is not None else [copy.deepcopy(mols[i])] for i, r in enumerate(results)]
587577

588578

@@ -615,8 +605,7 @@ def safe_index(items, element):
615605

616606

617607
def parse_receptor(pdbid, pdbbind_dir, dataset_type):
618-
rec = parsePDB(pdbid, pdbbind_dir, dataset_type)
619-
return rec
608+
return parsePDB(pdbid, pdbbind_dir, dataset_type)
620609

621610

622611
def parsePDB(pdbid, pdbbind_dir, dataset_type):

packages/matcha_nvmolkit_worker/src/matcha_nvmolkit_worker/cli.py

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ def _read_input_molecules(input_sdf: Path):
119119
molecules = []
120120
names = []
121121
valid_mask = []
122-
invalid_count = 0
123122
for idx, mol in enumerate(supplier):
124123
if mol is None:
125124
continue
@@ -133,41 +132,39 @@ def _read_input_molecules(input_sdf: Path):
133132
current.RemoveAllConformers()
134133
valid_mask.append(True)
135134
except Exception:
136-
invalid_count += 1
137135
valid_mask.append(False)
138136
molecules.append(current)
139137
names.append(uid)
138+
invalid_count = valid_mask.count(False)
140139
return molecules, names, valid_mask, invalid_count
141140

142141

143142
def _ensure_minimum_conformers(mol, confs_per_mol: int, seed: int | None):
144-
target = max(1, int(confs_per_mol))
145-
if mol.GetNumConformers() >= target:
143+
if mol.GetNumConformers() >= max(1, int(confs_per_mol)):
144+
return False
145+
if mol.GetNumConformers() > 0:
146146
return False
147147

148-
before = mol.GetNumConformers()
148+
conf = Chem.Conformer(mol.GetNumAtoms())
149+
for atom_idx in range(mol.GetNumAtoms()):
150+
conf.SetAtomPosition(atom_idx, Point3D(0.0, 0.0, 0.0))
151+
mol.AddConformer(conf, assignId=True)
152+
return True
149153

150-
if mol.GetNumConformers() == 0:
151-
conf = Chem.Conformer(mol.GetNumAtoms())
152-
for atom_idx in range(mol.GetNumAtoms()):
153-
conf.SetAtomPosition(atom_idx, Point3D(0.0, 0.0, 0.0))
154-
mol.AddConformer(conf, assignId=True)
155154

156-
return mol.GetNumConformers() != before
155+
def _conf_has_finite_coords(conf) -> bool:
156+
for atom_idx in range(conf.GetNumAtoms()):
157+
pos = conf.GetAtomPosition(atom_idx)
158+
if not (math.isfinite(pos.x) and math.isfinite(pos.y) and math.isfinite(pos.z)):
159+
return False
160+
return True
157161

158162

159163
def _drop_non_finite_conformers(mol):
160-
bad_ids = []
161-
for cid in range(mol.GetNumConformers()):
162-
conf = mol.GetConformer(cid)
163-
ok = True
164-
for atom_idx in range(conf.GetNumAtoms()):
165-
pos = conf.GetAtomPosition(atom_idx)
166-
if not (math.isfinite(pos.x) and math.isfinite(pos.y) and math.isfinite(pos.z)):
167-
ok = False
168-
break
169-
if not ok:
170-
bad_ids.append(cid)
164+
bad_ids = [
165+
cid for cid in range(mol.GetNumConformers())
166+
if not _conf_has_finite_coords(mol.GetConformer(cid))
167+
]
171168
for cid in reversed(bad_ids):
172169
mol.RemoveConformer(cid)
173170
return len(bad_ids)

0 commit comments

Comments
 (0)