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

Replace run_somd.sh with Pydantic configuration class #25

Conversation

fjclark
Copy link
Collaborator

@fjclark fjclark commented Jan 4, 2025

Description

This completes pretty much all the changes suggested in this comment: #11 (comment). All tests pass, although a3fe/tests/test_sys_prep_configuration.py::test_config_dump_and_load is flaky - it seems to fail when run as part of the full test suite. We should fix this before merging into main.

@Roy-Haolin-Du please could you take a look? Feel free to merge in if you're happy and fix any mistakes I've made. Pushing quickly to avoid any duplication of effort with you also working on it. Thanks!

This includes updates to the docs and a purge of run_somd.sh
from all the example directories. Note that a3fe/tests/test_sys_prep_configuration.py::test_config_dump_and_load
seems to be flaky - failing when run with all the tests, but fine
when run individually.
Copy link
Contributor

@Roy-Haolin-Du Roy-Haolin-Du left a comment

Choose a reason for hiding this comment

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

A brief explanation to avoid misunderstanding.
Very helpful method involved "analysis_slurm_config"
When 'analysis_slurm_config' is passed as an argument from process_grads.py, it is assigned to renamed as 'slurm_config' to do analyse in mbar.py.

@Roy-Haolin-Du Roy-Haolin-Du merged commit 3503aac into michellab:feature-decouple-somd-2 Jan 5, 2025
@fjclark
Copy link
Collaborator Author

fjclark commented Jan 6, 2025

Yes - the idea is store two SlurmConfig objects in all SimluationRunner objects. When running MD, the slurm_config attribute is used, and when doing analysis the SlurmConfig saved in the analysis_slurm_config attribute is used. Anything which runs MBAR is passed the analysis_slurm_config, which is the only slurm_config it knows about. There's a section in the docs which shows how it's used:

The molecular dynamics simulations should be run on GPUs - they are unbearably slow on CPU. However, you may want to run the MBAR analysis on CPUs to minimise submissions to the CPU queue. 
To do this, you can supply an ``analysis_slurm_config`` which is different to the ``slurm_config``, which will only be used for the MBAR analysis.

.. code-block:: python
    analysis_slurm_config = a3.SlurmConfig(
        partition="my-cluster-cpu-partition",
        time="00:10:00",
    )
    calc = a3.Calculation(
        slurm_config=slurm_config,
        analysis_slurm_config=analysis_slurm_config,
    )
  Then make sure to set ``slurm=True`` when running the analysis, e.g. ``calc.analyse(slurm=True)``.

Note that the slurm_config and analysis_slurm_config attrs of all sub-simulation runners just point to the corresponding attr of the top simulation runner (e.g. a calculation if you're just running one calculation), so you can change all the slurm options for all sub-simulation runners by doing e.g. calc.slurm_config.partition = "other_partition".

Hope that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants