[WIP] Minimal working, automated and tested examples

[ijpulidos]

Description

Removing old and obsolete examples. Leaving current atom-mapping notebook and cleaning up the protein mutation non-equilibrium switching API example.

Still missing protein-ligand and only ligand examples, for both CLI and API.

Motivation and context

Resolves #813

How has this been tested?

Just local tests so far (manually checking it runs and it gives reasonable output), need to write automated tests for this.

Change log

Minimal examples of protein, protein-ligand and ligand mutations, with automated testing.

[mikemhenry]

+162 −76,114 I love negative PRs

[ijpulidos]

Ok, I noticed that sometimes I get an openmm error as follows:

Traceback (most recent call last):
  File "/lila/home/pulidoi/workdir/repos/perses/examples/protein-neq-switching/run_neq_distrib_flattened.py", line 100, in <module>
    integrator.step(1)
  File "/home/pulidoi/miniconda3/envs/perses-dev/lib/python3.9/site-packages/simtk/openmm/openmm.py", line 7036, in step
    return _openmm.CustomIntegrator_step(self, steps)
simtk.openmm.OpenMMException: Particle coordinate is nan

I have the feeling has something to do with the platform, I have forced it to 'CPU'. Any ideas why this may be happening or if you have experienced similar issues?

[ijpulidos]

+162 −76,114 I love negative PRs

@mikemhenry Well, tests are failing and no idea why, haha. Maybe it wasn't such a great idea to remove all of this? Though I really cannot tell why the tests are failing that way, it works in my computer and in the HPC system.

[mikemhenry]

Locally I get these errors as well, for example:

____________________________________________________ test_examples[/home/mmh/Projects/perses/examples/protein-neq-switching/make_htf.py] _____________________________________________________
[gw3] linux -- Python 3.8.8 /home/mmh/miniconda3/envs/perses/bin/python

file_path = '/home/mmh/Projects/perses/examples/protein-neq-switching/make_htf.py', cmd_args = None

    def run_script_file(file_path, cmd_args=None):
        """Run through the shell a python script."""
        with tempfile.TemporaryDirectory() as tmp_dir:
            os.chdir(tmp_dir)
            cmd = ["python", file_path]
            # Extend cmd list with given cmd_args
            if cmd_args:
                cmd.extend(cmd_args)
            try:
>               subprocess.check_call(cmd)

/home/mmh/Projects/perses/perses/tests/test_examples.py:34: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

popenargs = (['python', '/home/mmh/Projects/perses/examples/protein-neq-switching/make_htf.py'],), kwargs = {}, retcode = 1
cmd = ['python', '/home/mmh/Projects/perses/examples/protein-neq-switching/make_htf.py']

    def check_call(*popenargs, **kwargs):
        """Run command with arguments.  Wait for command to complete.  If
        the exit code was zero then return, otherwise raise
        CalledProcessError.  The CalledProcessError object will have the
        return code in the returncode attribute.
    
        The arguments are the same as for the call function.  Example:
    
        check_call(["ls", "-l"])
        """
        retcode = call(*popenargs, **kwargs)
        if retcode:
            cmd = kwargs.get("args")
            if cmd is None:
                cmd = popenargs[0]
>           raise CalledProcessError(retcode, cmd)
E           subprocess.CalledProcessError: Command '['python', '/home/mmh/Projects/perses/examples/protein-neq-switching/make_htf.py']' returned non-zero exit status 1.

/home/mmh/miniconda3/envs/perses/lib/python3.8/subprocess.py:364: CalledProcessError

During handling of the above exception, another exception occurred:

example_file_path = '/home/mmh/Projects/perses/examples/protein-neq-switching/make_htf.py'

    @pytest.mark.parametrize("example_file_path", find_example_scripts())
    def test_examples(example_file_path):
        """Test that the example run without errors."""
>       run_script_file(example_file_path)

/home/mmh/Projects/perses/perses/tests/test_examples.py:61: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

file_path = '/home/mmh/Projects/perses/examples/protein-neq-switching/make_htf.py', cmd_args = None

    def run_script_file(file_path, cmd_args=None):
        """Run through the shell a python script."""
        with tempfile.TemporaryDirectory() as tmp_dir:
            os.chdir(tmp_dir)
            cmd = ["python", file_path]
            # Extend cmd list with given cmd_args
            if cmd_args:
                cmd.extend(cmd_args)
            try:
                subprocess.check_call(cmd)
            except subprocess.CalledProcessError:
>               raise Exception(f"Example {file_path} failed")
E               Exception: Example /home/mmh/Projects/perses/examples/protein-neq-switching/make_htf.py failed

/home/mmh/Projects/perses/perses/tests/test_examples.py:36: Exception
------------------------------------------------------------------------------------ Captured stderr call ------------------------------------------------------------------------------------
INFO:numexpr.utils:Note: NumExpr detected 16 cores but "NUMEXPR_MAX_THREADS" not set, so enforcing safe limit of 8.
INFO:numexpr.utils:NumExpr defaulting to 8 threads.
Traceback (most recent call last):
  File "/home/mmh/Projects/perses/examples/protein-neq-switching/make_htf.py", line 3, in <module>
    solvent_delivery = PointMutationExecutor("ala_vacuum.pdb",
  File "/home/mmh/Projects/perses/perses/app/relative_point_mutation_setup.py", line 169, in __init__
    protein_pdbfile = open(protein_filename, 'r')
FileNotFoundError: [Errno 2] No such file or directory: 'ala_vacuum.pdb'

Which looks like it was unable to find the ala_vacuum.pdb file

This is how I run the tests locally to simulate being on GHA:
GITHUB_ACTIONS=true CUDA_VISIBLE_DEVICES= pytest -v -a "not advanced" -n auto
That way some of the tests that we skip on GHA we skip and we don't use a GPU if there is one.

[ijpulidos]

@mikemhenry Thanks for letting me know how to run these tests simulating GHA.

By the way, I think you probably have a dirty examples/protein-neq-switching directory with some untracked .py files there, because there shouldn't be any make_htf.py. The test works by finding all the .py files in the examples directories.

[mikemhenry]

Good catch, I didn't clean that directory! The danger of dirty/untracked files and globbing :)

[ijpulidos]

Ok, so at least I made the test_examples.py work. I now have to think why we are getting this error in many of the other tests:

>           cwd = os.getcwd()
E           FileNotFoundError: [Errno 2] No such file or directory

Doesn't make much sense to me, but maybe something not getting cleaned up properly when the examples test is run with subprocess?

[ijpulidos]

@mikemhenry If you think about it, it makes sense. Since it is complaining now that it is not finding any neighboring waters (because vacuum). And it takes any word different from "vacuum" for phase keyword to actually solvate the system, as per

perses/perses/app/relative_point_mutation_setup.py

Line 542 in 59f47ec

if phase != 'vacuum':

. So now the question is if we actually want this in "vacuum" or not, and if not, then what to put there in the phase keyword (I guess not putting anything still works).

[codecov]

Codecov Report

Merging #844 (072bdd3) into master (de5397e) will increase coverage by 0.14%.
The diff coverage is 86.84%.

[mikemhenry]

I think we want a vacuum since that will be faster.

[ijpulidos]

I think we want a vacuum since that will be faster.

@mikemhenry please do note that the test has been running without vacuum this whole time. If we really need vacuum, I think the test has to be done differently, because I believe the assertion in

perses/perses/rjmc/topology_proposal.py

Line 586 in de5397e

assert len(nonneighboring_residues) > 0, "there are no available nonneighboring waters"

won't let it happen.

[zhang-ivy]

Ah good catch! It must have been running in the solvent phase since vacuum was spelled wrong.

We should probably just change the test case to a different mutant residue (i.e. instead of mutating from ALA to ASP, mutate from ALA to THR (or any non-charged amino acid). We won't be able to do charge changing transformations in vacuum because charge changing mutations require transforming water into an ion to account for the charge change. In vacuum, there are no waters present to allow for this.

[ijpulidos]

@zhang-ivy Yes, that sounds good to me. I'll be making the changes and hopefully that will do it for this PR.

[ijpulidos]

One of the tests is failing but for a whole new different reason. There is definitely a syntax error here, that stop is not something in Python :). Now I don't know why we are hitting that in this case, I guess it is one of these things that you hit only some times? Other tests runs were executed just fine.

@jchodera Do we want that to be an actual pass, that is, it is okay to have an InvalidMappingException in that case? If that's the case then this fix is pretty trivial. If not, then we should think of whether we want to raise an error there or not. Could you please review this? Thanks.

[mikemhenry]

@ijpulidos it would be nice to remove that stop and just see what the error is, that would help us figure out what we want to do here
EDIT: like raise e

[ijpulidos]

@mikemhenry So yeah, I was afraid this would happen. It is not hitting the exception/error now. It is something that does not happen every time, apparently. I guess we can merge this and raise an issue separately for it.

[mikemhenry]

We should catch it on our nightly CI eventually -- let's wait until we hear from @jchodera with what his intention was with stop and if we want to raise the error or not. I think since tests are passing, we probably want to raise the error, but we should see what he says.

[mikemhenry]

🎉 great work @ijpulidos Having examples that we know will work will make testing things (as well as brining new users to perses) much easier! Just had a few minor nits!

[mikemhenry]

@ijpulidos looks good, auto merge is on!

[jchodera]

One of the tests is failing but for a whole new different reason. There is definitely a syntax error here, that stop is not something in Python :).

Whoops! I must have put that in accidentally as part of debugging. The stop shouldn't be there---it was just something meant to hit a "poor man's breakpoint" for further interactive debugging.