Processor multiprocessing #233
Conversation
|
It seems we need to add a test for |
There was a problem hiding this comment.
Some small comments. The downside of this approach is that every Processor being run creates it's own Pool of processes, which might create a lot of overhead. I'm not sure how bad it is though.
Possible alternative is to have the entry point (/bin/martinize2, the CLI) create a Pool that's given to every Processor (None default).
Still missing in this PR is the CLI flag.
@jbarnoud I need a second opinion on this. What's the best way of passing the multiprocessing option/choices to all processors? My current idea (that I conveyed to @mjtadema) is to simply set a vermouth.processors.processor.Processor class attribute, rather than muck around with __init__ and instance attributes.
|
|
||
| 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 |
There was a problem hiding this comment.
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.
| for molecule in system.molecules: | ||
| mols.append(self.run_molecule(molecule)) | ||
| system.molecules = mols | ||
| if hasattr(self, 'nproc') and self.nproc > 1: |
There was a problem hiding this comment.
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.
Instead of setting the class attribute |
|
Yes, that would be an option. |
|
stale PR; also it's so far behind it will probably be easier to make a new one. |
This is what i came up with for multiprocessing in the base run_system method.
It will always fall back to the old method if
nprocis not set.So far it seems to work fine in combination with my changed make_bonds function in #234 .