WIP: Custom grids for all biases
Leverages the grid refactoring from #710.
Implements #476
Is it possible to use multiple grids in a bias? I am not sure if the configuration syntax in PR #710 implies that there is only one grid allowed.
Is it possible to use multiple grids in a bias? I am not sure if the configuration syntax in PR #710 implies that there is only one grid allowed.
Since the parsing of the grid { ... } options is done with key_lookup() there is no restriction against defining multiple ones.
This long-lived branch carries a high risk of conflicts. I recommend merging this in this state (incomplete but functional and self-consistent) and completing the features in a separate PR.
@jhenin I think this can be finished soon; metadynamics is pretty much done.
I got two small issues:
- To make things faster, I added as optional parameter the config string, but this has the downside that the parsing is done multiple times for related objects (i.e. the biasing energy and its gradients). I like how you used the new base class
colvar_grid_paramsin ABF to avoid that. To generalize this to other cases, how about allocating directly (usingunique_ptr) an object of that class to store the grid definition? - Parsing "width" and "widths" at the same time is very confusing for users: I silenced the one that is only used in state files. However, I really think that the configuration parsing and state file reading should be separate functions now.
To make things faster, I added as optional parameter the config string, but this has the downside that the parsing is done multiple times for related objects (i.e. the biasing energy and its gradients). I like how you used the new base class colvar_grid_params in ABF to avoid that. To generalize this to other cases, how about allocating directly (using unique_ptr) an object of that class to store the grid definition?
It's not completely clear to me how the other use cases are different from ABF to the extent that the approach I used there doesn't work, but if that's the case, I have no issue with instantiating colvar_grid_params on its own.
Parsing "width" and "widths" at the same time is very confusing for users: I silenced the one that is only used in state files. However, I really think that the configuration parsing and state file reading should be separate functions now.
I agree. Can this be a separate refactoring PR to avoid feature creep here? Except if you think it can be a minor addition.
It's not completely clear to me how the other use cases are different from ABF to the extent that the approach I used there doesn't work, but if that's the case, I have no issue with instantiating colvar_grid_params on its own.
NVM, you had used another derived class (colvar_grid_count) to initialize a colvar_grid_gradient object, but it's the same principle.
I agree. Can this be a separate refactoring PR to avoid feature creep here? Except if you think it can be a minor addition.
Since I am in the middle of it I can go either way, but the metadynamics output looks really weird right now (same parameters being parsed twice). To the extent that is feasible, we should keep things clear for those who are actually checking the output at initialization time.
NVM, you had used another derived class (
colvar_grid_count) to initialize acolvar_grid_gradientobject, but it's the same principle.
In the interest of clarity, you added what needs to be done for colvar_grid_count and used that to initialize colvar_grid_gradient. IMO, both should be initialized in the same way, from their shared parameters.
Sounds good to me.
@jhenin I think this is it regarding the code. Are you still up for writing the documentation? https://github.com/Colvars/colvars/pull/468#issuecomment-1117410949
Adding both as reviewers for the code portion. Most of the changes were straightforward, and the tests already caught a few mistake I made along the way. Still, there maybe some overlooked use cases.
@giacomofiorin I did agree to write the doc. That was a long time ago: I was young and foolish. I'll do it.
Gromacs-main broke Colvars again:
gmx_d mdrun -ntmpi 4 -s test.restart.tpr -ntomp 2 -deffnm test.restart -noappend -cpi test.cpt
-------------------------------------------------------
Program: gmx mdrun, version 2026.0-Colvars_2025_04_07-dev-20250416-f1d17c175a-dirty
Standard library logic error (bug):
(exception type: St12out_of_range)
unordered_map::at
I ran the tests of slightly older Gromacs on my machine locally. Since this PR is no less or more broken than master, I'd say it can be merged, and we worry about fixing Gromacs-main in a separate branch.