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

hat_reaction changing kmc type not intuitive #385

Open emerbitz opened 8 months ago

emerbitz commented 8 months ago

From a user perspective it is confusing that the kmc type is changed by a reaction plugin. I think it should be a setting in the kimmdy config.yml, not of the individual plugins. (written by Eric)

Any thought on this?

KRiedmiller commented 8 months ago

If I'm not mistaken, it is not 'changed' by the plugin, it is set in the first place by the plugin. If the user wants, he can overwrite the choice in the kimmdy.yml, either in the root of it or per plugin.

So, if you set the algorithm in the root of the config, this also should be used.
It is documented as "KMC algorithm overwrite. Should be set by the reactions, but can be changed here."

If this choice is not honored, I'd consider it a bug

ehhartmann commented 8 months ago

Effectively the plugin changes the behavior of the kimmdy run because decide_recipe is outside of the plugin. For Eddie this was unexpected behavior and I agree. The same applies for apply_recipe but I think this is closer related to the plugin than decide_recipe. I would be fine with logging a info/warning in the plugin if the chosen kmc type is not optimal for the reaction but would prefer not to set the kmc type. The same behavior could be achieved by making the kmc type a required input in the config.yml without providing a default (to make the user think or at least see the kmc type). The kmc type is a major setting in a kimmdy run and should be more visible.

ehhartmann commented 8 months ago

Write whole config object to json in 0_setup?

ehhartmann commented 8 months ago

make plugin documentation accessible thorugh kimmdy documentation?