michellab / BioSimSpace

Code and resources for the EPSRC BioSimSpace project.
https://biosimspace.org
GNU General Public License v3.0
77 stars 19 forks source link

Water name is not correctly converted in vacuum simulation #338

Closed xiki-tempula closed 2 years ago

xiki-tempula commented 2 years ago

Prerequisites

This might be an edge case as I'm lazy and find this issue when trying to scavenge some of the existing test.

In [1]: import BioSimSpace.Sandpit.Exscientia as BSS

In [2]: system = BSS.IO.readMolecules("test/input/amber/ala/*")

In [3]: m0 = system.getMolecule(0)

In [4]: m1 = system.getMolecule(1)

In [5]: new_system = (m0+m1).toSystem()

In [6]: protocol = BSS.Protocol.Minimisation(steps=100)

In [7]: process = BSS.Process.Gromacs(new_system, protocol, name="test")
/Users/zwu/Documents/GitHub/BioSimSpace/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py:247: UserWarning: No simulation box found. Assuming gas phase simulation.
  _warnings.warn("No simulation box found. Assuming gas phase simulation.")
/Users/zwu/Documents/GitHub/BioSimSpace/python/BioSimSpace/Sandpit/Exscientia/Protocol/_config.py:37: UserWarning: No simulation box found. Assuming gas phase simulation.
  _warnings.warn("No simulation box found. Assuming gas phase simulation.")
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
<ipython-input-7-7012fab1aba9> in <module>
----> 1 process = BSS.Process.Gromacs(new_system, protocol, name="test")

~/Documents/GitHub/BioSimSpace/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py in __init__(self, system, protocol, exe, name, work_dir, seed, extra_options, extra_lines, property_map, restraint, ignore_warnings, show_errors)
    171 
    172         # Now set up the working directory for the process.
--> 173         self._setup()
    174 
    175     def _setup(self):

~/Documents/GitHub/BioSimSpace/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py in _setup(self)
    224             self.setConfig(self._protocol.getConfig())
    225         else:
--> 226             self._generate_config()
    227         self.writeConfig(self._config_file)
    228 

~/Documents/GitHub/BioSimSpace/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py in _generate_config(self)
    758         config = _Protocol.ConfigFactory(self._system, self._protocol)
    759         self.addToConfig(config.generateGromacsConfig(extra_options={**config_options, **self._extra_options},
--> 760                                                       extra_lines=self._extra_lines))
    761 
    762         # Flag that this isn't a custom protocol.

~/Documents/GitHub/BioSimSpace/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py in addToConfig(self, config)
    856 
    857         # Use grompp to generate the portable binary run input file.
--> 858         self._generate_binary_run_file()
    859 
    860     def resetConfig(self):

~/Documents/GitHub/BioSimSpace/python/BioSimSpace/Sandpit/Exscientia/Process/_gromacs.py in _generate_binary_run_file(self)
    835                                      +  "\nUse 'ignore_warnings' to ignore warnings."
    836 
--> 837                 raise RuntimeError(exception_string)
    838 
    839             else:

RuntimeError: Unable to generate GROMACS binary run input file.

'gmx grompp' reported the following warnings:
WARNING 1 [file test.top, line 258]:
  3 non-matching atom names
  atom names from
  /var/folders/qr/hdc8n1_j0ll6ccs2dl4_5r700000gq/T/tmp1hxwtcgx/test.top
  will be used
  atom names from
  /var/folders/qr/hdc8n1_j0ll6ccs2dl4_5r700000gq/T/tmp1hxwtcgx/test.gro
  will be ignored

Use 'ignore_warnings' to ignore warnings.

So the water name is

    4WAT      O   23   2.785562   2.558994   2.243299
    4WAT     H1   24   2.777503   2.467540   2.216216
    4WAT     H2   25   2.744207   2.608665   2.172696
lohedges commented 2 years ago

Would there be a situation where you want to simulate lone water molecule in vacuum? I can certainly check for water molecules and do the conversion in all cases, but just wanted to check if this was a situation that someone would come across in practice? (That said, I imagine there could be issues if crystal waters were present, but that's an issue for another time.)

xiki-tempula commented 2 years ago

@lohedges I think this is more like how one organise the code and where should the system._set_water_topology be called.

Do we expect the everything (topology and beyond) to be modified based whether this is in vacuum or not. Or the only difference between the in vacuum or not is strictly the box size and the cutoff.

lohedges commented 2 years ago

Ah, I see the issue. While set_water_topology is called at the start of the setup, i.e. before any protocol specific configuration things are implemented, the original system (without modified water topology) is used to write the modified gro file, which is needed in the case of vacuum simulations, i.e. to add an infinite periodic box. This problem is specific to GROMACS.

I'll fix this when I get a moment.

Thanks for catching!