Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Processor multiprocessing #233

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Processor multiprocessing #233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
98033ea
Added multiprocessing to run_system method of Processor class
Mar 4, 2020
eeed05f
Fixed checking for the nproc attribute
Mar 4, 2020
bb84d56
Setting nproc in the class attribute now
Mar 4, 2020
08c6a7e
Added a test for multiprocessing
Mar 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions vermouth/processors/processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@
"""
Provides an abstract base class for processors.
"""

import multiprocessing

class Processor:
"""
An abstract base class for processors. Subclasses must implement a
`run_molecule` method.
Has nproc attribute that is changed by a CLI flag
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

Attributes
------------
nproc: int
    Describes how many CPUs this processor can use.

No need to mention the CLI here. It could also come from different code, a web interface, whatever.

"""
nproc = 1
def run_system(self, system):
"""
Process `system`.
Expand All @@ -32,10 +34,14 @@ def run_system(self, system):
system: vermouth.system.System
The system to process. Is modified in-place.
"""
mols = []
for molecule in system.molecules:
mols.append(self.run_molecule(molecule))
system.molecules = mols
if hasattr(self, 'nproc') and self.nproc > 1:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasattr check is not needed, since every object is guaranteed to have one.
Also, add an extra check around here that if nproc==-1 (or 0, or None, or whatever), None will be passed to Pool, making it use all CPUs.

pool = multiprocessing.Pool(self.nproc)
system.molecules = pool.map(self.run_molecule, system.molecules)
else:
mols = []
for molecule in system.molecules:
mols.append(self.run_molecule(molecule))
system.molecules = mols

def run_molecule(self, molecule):
"""
Expand Down
12 changes: 12 additions & 0 deletions vermouth/tests/test_make_bonds.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,21 @@ def test_make_bonds(nodes, edges, expected_edges):
mol.add_nodes_from(enumerate(node_set))
mol.add_edges_from(edge_set)
system.add_molecule(mol)
system_mp = system.copy()

MakeBonds().run_system(system)
# Make sure number of connected components is the same
assert len(system.molecules) == len(expected_edges)
# Make sure that for every molecule found, the edges are correct
for found_mol, ref_edges in zip(system.molecules, expected_edges):
assert dict(found_mol.edges) == ref_edges

# Also test making bonds with multiprocessing
mb = MakeBonds()
mb.nproc = 2
mb.run_system(system)
# Make sure number of connected components is the same
assert len(system.molecules) == len(expected_edges)
# Make sure that for every molecule found, the edges are correct
for found_mol, ref_edges in zip(system.molecules, expected_edges):
assert dict(found_mol.edges) == ref_edges