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.

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

Use srun by default in CP2K #90

Merged
merged 4 commits into from
Jun 17, 2020
Merged

Use srun by default in CP2K #90

merged 4 commits into from
Jun 17, 2020

Conversation

felipeZ
Copy link
Contributor

@felipeZ felipeZ commented Jun 17, 2020

fix #86

@felipeZ felipeZ requested a review from BvB93 June 17, 2020 13:44
@BvB93
Copy link
Collaborator

BvB93 commented Jun 17, 2020

While you're at it, could you add an else statement after the for / break loop?

If the break statement is never reached then it will get stuck on command = "mpirun", even though mpirun is unavailable in such case. This is guaranteed to crash the job down the line.

for command in ('srun', 'mpirun'):
    try:
        ret = ...
    except OSError:
        break
else:  # In case the previous `break` statement is never reached
    ret = 'sopt'

@BvB93 BvB93 linked an issue Jun 17, 2020 that may be closed by this pull request
@felipeZ
Copy link
Contributor Author

felipeZ commented Jun 17, 2020

@BvB93 you are write :) I have clean a bit the code and allow the user to invoke her/his own call to MPI using srun or mpirun

Copy link
Collaborator

@BvB93 BvB93 left a comment

Choose a reason for hiding this comment

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

Looking pretty good.
I do feel that, at this point, it might be a good idea to update the method docstring:

"""Run CP2K.

The exact CP2K executable, and whether or not one wants to use ``srun`` or ``mpirun``, can be specified under :code:`self.settings.executable`. 
Currently supported executables are:

* ``"sdbg"``: Serial single core testing and debugging
* ``"sopt"``: Serial general single core usage
* ``"ssmp"``: Parallel (only OpenMP), single node, multi core
* ``"pdbg"``: Parallel (only MPI) multi-node testing and debugging
* ``"popt"`` (default): Parallel (only MPI) general usage, no threads
* ``"psmp"``: parallel (MPI + OpenMP) general usage, threading might improve scalability and memory usage

For example:

.. code-block:: python

    >>> from scm.plams import Cp2kJob

    >>> job = Cp2kJob(...)
    >>> job.settings.executable = "popt"
    >>> job.settings.executable = "ssmp"
    >>> job.settings.executable = "mpirun psmp"

"""

interfaces/thirdparty/cp2k.py Outdated Show resolved Hide resolved
interfaces/thirdparty/cp2k.py Outdated Show resolved Hide resolved
@felipeZ felipeZ merged commit c71d4cc into master Jun 17, 2020
@felipeZ felipeZ deleted the srun_cp2k branch June 17, 2020 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use srun for cp2k with MPI
2 participants