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

KIMMDY has problems with molecules that are not in amberff99sb #359

Open
ehhartmann opened this issue Jan 31, 2024 · 9 comments
Open

KIMMDY has problems with molecules that are not in amberff99sb #359

ehhartmann opened this issue Jan 31, 2024 · 9 comments
Labels

Comments

@ehhartmann
Copy link
Collaborator

At least two areas in the code still need information from aberff99sb:

  1. The Topology module uses aminoacids.rtp to use the residuetypes (e.g. for a partial charge lookup after a binding event)
  2. When checking for radicals, we use the bondorder list with amber atom types (e.g. in the HAT plugin and topology module)

One can get around most (all?) issues by manually editing the topology and aminoacids.rtp file but it would be much more convenient to rewrite those parts.

Proposed solution for 1:

  • expose the filename that contains residuetypes in the kimmdy.yml
    or - find ways to do all the tasks without using residuetypes

Proposed solution for 2:

  • Add the option to list starting structure radical atom nrs in kimmdy.yml and make sure kimmdy correctly keeps track of radicals after reactions

Opinions/Ideas @KRiedmiller @jmbuhr ?

@jmbuhr
Copy link
Collaborator

jmbuhr commented Jan 31, 2024

  1. I like exposing the config for the file with the residuetypes, or a list of files in case there are multiple (e.g. aminoacids.rtp and lipid.rtp in charrm36). I don't think we can get around parsing those, because we need the information specifically about improper dihedrals as those can't be inferred just from the atoms.
  2. Or a expose the config for a file with a list of atomtypes and their bondorder? Or can we infer this from the forcefield somehow? I think it would be nice for kimmdy to retain the capability to determine radicals and not having to rely on being told about them at the start.

@ehhartmann
Copy link
Collaborator Author

How about we write a tool that tries to find the radicals and have a csv file or list of atom nrs as option in the config file?

@KRiedmiller
Copy link
Collaborator

How about:

image

@ehhartmann
Copy link
Collaborator Author

Could we have a single residuetypes file that has includes instead of a list of residuetypes files?

@jmbuhr
Copy link
Collaborator

jmbuhr commented Jan 31, 2024

kimmdy could, since we use the same parser for rtp as for top and itp files, but I don't know if gromacs handles those differently.

@jmbuhr
Copy link
Collaborator

jmbuhr commented Jan 31, 2024

maybe gromacs just reads all the .rtp files that it finds when pdb2gmx is used

@ehhartmann
Copy link
Collaborator Author

Fixed in #360

@ehhartmann
Copy link
Collaborator Author

refactor reintroduced error when handling non-amber molecules

Traceback (most recent call last):
  File "/hits/fast/mbm/hartmaec/sw/conda/envs/kimmdy_full/bin/kimmdy", line 8, in <module>
    sys.exit(entry_point_kimmdy())
  File "/hits/fast/mbm/hartmaec/kimmdys/kimmdy/src/kimmdy/cmd.py", line 347, in entry_point_kimmdy
    _run(args)
  File "/hits/fast/mbm/hartmaec/kimmdys/kimmdy/src/kimmdy/cmd.py", line 290, in _run
    raise e
  File "/hits/fast/mbm/hartmaec/kimmdys/kimmdy/src/kimmdy/cmd.py", line 281, in _run
    runmgr.run()
  File "/hits/fast/mbm/hartmaec/kimmdys/kimmdy/src/kimmdy/runmanager.py", line 215, in run
    next(self)
  File "/hits/fast/mbm/hartmaec/kimmdys/kimmdy/src/kimmdy/runmanager.py", line 313, in __next__
    files = task()
  File "/hits/fast/mbm/hartmaec/kimmdys/kimmdy/src/kimmdy/tasks.py", line 152, in __call__
    return self.f(**self.kwargs)
  File "/hits/fast/mbm/hartmaec/kimmdys/kimmdy/src/kimmdy/runmanager.py", line 619, in _apply_recipe
    top_merge = merge_top_slow_growth(top_initial, deepcopy(self.top))
  File "/hits/fast/mbm/hartmaec/kimmdys/kimmdy/src/kimmdy/coordinates.py", line 498, in merge_top_slow_growth
    molB = merge_top_moleculetypes_slow_growth(molA, molB, topB.ff, focus_nr)
  File "/hits/fast/mbm/hartmaec/kimmdys/kimmdy/src/kimmdy/coordinates.py", line 482, in merge_top_moleculetypes_slow_growth
    molB.find_radicals()
  File "/hits/fast/mbm/hartmaec/kimmdys/kimmdy/src/kimmdy/topology/topology.py", line 249, in find_radicals
    bo = ATOMTYPE_BONDORDER_FLAT[atom.type]
KeyError: 'c3'

will check whether
File "/hits/fast/mbm/hartmaec/kimmdys/kimmdy/src/kimmdy/coordinates.py", line 482, in merge_top_moleculetypes_slow_growth molB.find_radicals()
find_radicals() call is really necessary. Should test whether our list of radicals is always trustworthy and in sync with atom.is_radical

@ehhartmann
Copy link
Collaborator Author

relates to #404

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

No branches or pull requests

3 participants