Set target temperature through a common code path #513

giacomofiorin · 2023-01-25T12:11:41Z

Minimize divergences between MD engines by storing the target temperature in a proxy member and provide a shared function set_target_temperature() to set it. The previous virtual function temperature() is now a const accessor target_temperature().

Additionally, there is now a script method ~~settemperature~~ targettemperature to set it in scripts, i.e. in post-processing or when we are temporarily unable to get it from the MD engine (see e.g. #218).

~~Questions:~~

~~Should set_target_temperature() be virtual or just remain a plain setter?~~
~~Name of the script function (settemperature -> settargettemperature`)?~~

jhenin · 2023-01-25T13:12:27Z

Should set_target_temperature() be virtual or just remain a plain setter?

Trying to think of a possible use for an override... The only one that comes to mind is an annealing method that would be driven from Colvars. That seems distant and unlikely, so I'd keep a plain setter for now, for simplicity. If the need arises to override it, making it virtual will be an easy change.

Name of the script function (settemperature -> settargettemperature)?

I think a getter would be useful too. The function could be called targettemperature and be a getter/setter like cv frame.

May be useful to dynamically redefine unit system, but at the moment other parameters need redefinition, e.g. angstrom_value

HubLot

I just have 2 comments but otherwise it's looks good on the gromacs side

gromacs/gromacs-2022.x/colvarproxy_gromacs.cpp

HubLot · 2023-01-26T10:40:53Z

gromacs/gromacs-2022.x/colvarproxy_gromacs.cpp

+  // From Gnu units
+  // $ units -ts 'k' 'kJ/mol/K/avogadro'
+  // 0.0083144621
+  boltzmann_ = 0.0083144621;


It could be worth using the GROMACS constant here: see gmx::c_boltz

I know we always write the number before but we could benefit from the PR

@HubLot I couldn't the definition of c_boltz anywhere. Is it auto-generated? I tried guessing from what similar source files include.

I tried but...

okay so:

for 2020 & 2021: #include "gromacs/math/units.h" with BOLTZ as a name (without gmx::)

for 2022: #include "gromacs/math/units.h" with gmx::c_boltz

didn't catch they changed the constant name in 2022 and above :)

Okay this works. Thanks!

jhenin · 2023-01-27T14:35:15Z

Trying VMD...

vmd > cv targettemperature
9.04115e+271

Hmmm... This will need a little fix.

and remove now unused backend_angstrom_value()

and kcal_mol_value, which is unused but preserved for completeness

jhenin · 2023-01-27T15:36:52Z

Force-pushed an update because of malicious behavior of multi-file search in VS Code, intentionally missing Gromacs files.

jhenin · 2023-01-26T12:59:13Z

src/colvarmodule.cpp

@@ -2107,18 +2107,6 @@ int cvm::load_coords_xyz(char const *filename,
 // Wrappers to proxy functions: these may go in the future


"these may go in the future": Is this the right time to get rid of the other wrapper functions below?

Absolutely, but in another PR. #487 is an example where tidying things up ended up in the way of finishing a specific task.

giacomofiorin · 2023-01-27T17:49:19Z

Hmmm... This will need a little fix.

...or not. Remember that in VMD it is the caller's responsibility to initialize class members? :-)

jhenin · 2023-01-27T18:06:26Z

Remember that in VMD it is the caller's responsibility to initialize class members? :-)

@jhenin

This update consists exclusively of bugfixes or maintenance-related changes. The following is a list of pull requests in the Colvars repository since the previous update to LAMMPS: - 532 Add XYZ trajectory reading feature Colvars/colvars#532 (@jhenin, @giacomofiorin) - 531 Delete objects quietly, unless explicitly requested via script (including VMD) Colvars/colvars#531 (@giacomofiorin) - 530 Append newline to log and error messages if not already present Colvars/colvars#530 (@giacomofiorin) - 528 Forward-declare OpenMP lock Colvars/colvars#528 (@giacomofiorin) - 527 Remove unneeded STL container Colvars/colvars#527 (@giacomofiorin) - 526 Allow collecting configuration files and strings before setting up interface Colvars/colvars#526 (@giacomofiorin, @jhenin) - 523 Fallback to linearCombination when customFunction is missing in customColvar Colvars/colvars#523 (@HanatoK, @giacomofiorin) - 522 Use iostream::fail() to check for I/O error Colvars/colvars#522 (@jhenin) - 520 Fix ref count Colvars/colvars#520 (@giacomofiorin) - 513 Set target temperature through a common code path Colvars/colvars#513 (@giacomofiorin, @jhenin) - 509 Safer detection of Windows with recent Microsoft Visual Studio versions Colvars/colvars#509 (@akohlmey) - 508 Update LAMMPS patching method to reflect Lepton availability Colvars/colvars#508 (@giacomofiorin) - 497 Increase the precision of write_multicol Colvars/colvars#497 (@HanatoK) - 496 Only perform MTS automatic enable/disable for timeStepFactor > 1 Colvars/colvars#496 (@giacomofiorin) - 493 Remove unused branch of quaternion input function Colvars/colvars#493 (@giacomofiorin) - 489 Ensure there are spaces between the fields in the header Colvars/colvars#489 (@HanatoK) - 487 Use map of output streams, and return references to its elements Colvars/colvars#487 (@giacomofiorin, @jhenin) - 486 Remember first step of moving restraint Colvars/colvars#486 (@jhenin) - 485 Add decoupling option for moving restraints Colvars/colvars#485 (@jhenin) - 483 Update Lepton via patching procedure Colvars/colvars#483 (@giacomofiorin) - 481 Make file-reading operations of input data abstractable Colvars/colvars#481 (@giacomofiorin) Authors: @akohlmey, @giacomofiorin, @HanatoK, @jhenin

giacomofiorin added 7 commits January 25, 2023 06:28

Split off colvarproxy_system class

26c4076

Remove redundant virtual functions from VMD proxy

538c989

Unify mechanisms to set target temperature

01e1850

Add method to set temperature from script

ab5e354

Update non-CMake build recipes

1a46710

Add a few overrides

9ae44b9

Fix missing declaration

d6702d2

giacomofiorin added 3 commits January 25, 2023 08:30

Remove derived version of function no longer available

2e79699

Make targettemperature a getter/setter

435eff5

Use directly proxy to get Boltzmann constant and temperature

070c042

giacomofiorin force-pushed the update_temperature branch from 9f66b59 to 070c042 Compare January 25, 2023 16:03

giacomofiorin added 6 commits January 25, 2023 12:16

Allow redefining Boltzmann factor

0e6f4f4

May be useful to dynamically redefine unit system, but at the moment other parameters need redefinition, e.g. angstrom_value

Update pre-MDModules GROMACS interfaces

d117f7f

Fix typo

d44b351

Update scripting doc

a7622e4

Finish removing virtual function no longer derived

17eb635

Use Boltzmann constant from LAMMPS

729cc85

giacomofiorin marked this pull request as ready for review January 25, 2023 17:55

giacomofiorin requested review from HubLot and jhenin January 25, 2023 17:56

HubLot reviewed Jan 26, 2023

View reviewed changes

Use Boltzmann constant from GROMACS

fb165ee

giacomofiorin force-pushed the update_temperature branch from 8742d86 to fb165ee Compare January 26, 2023 19:23

giacomofiorin added 2 commits January 26, 2023 14:45

Guess header where GROMACS physical constants are defined

0215105

Fix name of Boltzmann constant in older GROMACS versions

072fb84

jhenin added 2 commits January 27, 2023 15:52

Initialize targettemperature

1bed496

and remove now unused backend_angstrom_value()

Make angstrom_value protected

61e38cf

and kcal_mol_value, which is unused but preserved for completeness

jhenin force-pushed the update_temperature branch from c852bdf to 61e38cf Compare January 27, 2023 15:35

jhenin approved these changes Jan 27, 2023

View reviewed changes

jhenin closed this Jan 27, 2023

jhenin reopened this Jan 27, 2023

jhenin merged commit df9fb2e into master Jan 31, 2023

giacomofiorin deleted the update_temperature branch May 1, 2023 15:04

giacomofiorin mentioned this pull request May 17, 2023

Update Colvars library to version 2023-05-01 lammps/lammps#3783

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set target temperature through a common code path #513

Set target temperature through a common code path #513

giacomofiorin commented Jan 25, 2023 •

edited

jhenin commented Jan 25, 2023

HubLot left a comment

HubLot Jan 26, 2023

HubLot Jan 26, 2023

giacomofiorin Jan 26, 2023

giacomofiorin Jan 26, 2023

HubLot Jan 26, 2023

HubLot Jan 26, 2023

giacomofiorin Jan 26, 2023

jhenin commented Jan 27, 2023

jhenin commented Jan 27, 2023

jhenin Jan 26, 2023

giacomofiorin Jan 27, 2023

giacomofiorin commented Jan 27, 2023

jhenin commented Jan 27, 2023

		@@ -2107,18 +2107,6 @@ int cvm::load_coords_xyz(char const *filename,
		// Wrappers to proxy functions: these may go in the future

Set target temperature through a common code path #513

Set target temperature through a common code path #513

Conversation

giacomofiorin commented Jan 25, 2023 • edited

jhenin commented Jan 25, 2023

HubLot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhenin commented Jan 27, 2023

jhenin commented Jan 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giacomofiorin commented Jan 27, 2023

jhenin commented Jan 27, 2023

giacomofiorin commented Jan 25, 2023 •

edited