graeter-group / kimmdy

Reactive MD pipeline for GROMACS using Kinetic Monte Carlo / Molecular Dynamics (KIMMDY)
https://graeter-group.github.io/kimmdy/
GNU General Public License v3.0
8 stars 1 forks source link

Homolysis doesn't use updated topology #178

Closed KRiedmiller closed 1 year ago

KRiedmiller commented 1 year ago

Example Hexala: Kimmdy wants to break same bond over and over again
Since after the first break the 'bond' is really stressed, rates are through the roof afterwards

jmbuhr commented 1 year ago

So the topology ends up in topol_mod, but is never used? We also have a duplication between https://github.com/hits-mbm-dev/kimmdy/blob/a096a45af9a3cd0f7974301a307265404b105424/src/kimmdy/runmanager.py#L359 and https://github.com/hits-mbm-dev/kimmdy/blob/a096a45af9a3cd0f7974301a307265404b105424/src/kimmdy/changemanager.py#L161

I think we can simplify this a bunch. Because KIMMDY is the only thing making changes to the topology, the internal topology object should be the single source of truth. No task should actually read a top file again and a topology should only be written to a file for reading with an external program (gromacs). I wonder if https://github.com/hits-mbm-dev/kimmdy/blob/a096a45af9a3cd0f7974301a307265404b105424/plugins/src/homolysis/reaction.py#L34C7-L34C7 doesn't point to the modified topology? Either way, it could be replaced with https://github.com/hits-mbm-dev/kimmdy/blob/a096a45af9a3cd0f7974301a307265404b105424/plugins/src/hat_naive/reaction.py#L31

ehhartmann commented 1 year ago

I think this is a problem with plumed.dat

KRiedmiller commented 1 year ago

Indeed, the plumed_mod.dat contains the already broken atoms

ehhartmann commented 1 year ago

Fixed by #188

KRiedmiller commented 1 year ago

Nope, not fixed. Plumed seems correct now, but probably an issue with .dat being an ambiguous suffix

ehhartmann commented 1 year ago
        if "plumed" in md_config.get_attributes():
            mdrun_cmd += f" -plumed {md_config.plumed.dat}"
            files.output = {"plumed.dat": md_config.plumed.dat}
KRiedmiller commented 1 year ago

The issue is that get_latest is not used, therefore the plumed_mod is never used, only the original one from the config. Fixing it in my plumed rework currently. #216

jmbuhr commented 1 year ago

I believe I found another related bug where updated dihedrals are not properly reflected in the dictionary representation (used for writing to the file). Working on it over at https://github.com/hits-mbm-dev/kimmdy/pull/211 as well

KRiedmiller commented 1 year ago

fixed in #216