colvars icon indicating copy to clipboard operation
colvars copied to clipboard

Comparable Colvars version

Open HanatoK opened this issue 7 months ago • 5 comments

Currently version of Colvars is set by the colvars_version.h:

https://github.com/Colvars/colvars/blob/ec89df12686346d921788a5832231b18f1589143/src/colvars_version.h#L1-L3

It is a bit difficult to compare different Colvars version strings and compile external C++ code conditionally based on different versions of Colvars. I am a bit worried that by introducing https://github.com/Colvars/colvars/pull/788, any 3rd C++ code (for example, those code in https://www.ks.uiuc.edu/Training/Tutorials/#namd:~:text=Customizing%20CVs%20in%20NAMD%3A) relying on the Colvars internal APIs won't work, so it would be better to provide a way to conditionally compile C++ code based on the Colvars version.

I propose the following solution:

#ifndef COLVARS_VERSION

#include <cstdint>

#define COLVARS_VERSION_YEAR 2025
#define COLVARS_VERSION_MONTH 05
#define COLVARS_VERSION_DATE  12

#define COLVARS_VERSION_STR_HELPER(x) #x
#define COLVARS_VERSION_STR(x) COLVARS_VERSION_STR_HELPER(x)

#define COLVARS_VERSION \
        COLVARS_VERSION_STR(COLVARS_VERSION_YEAR) "-"\
        COLVARS_VERSION_STR(COLVARS_VERSION_MONTH) "-"\
        COLVARS_VERSION_STR(COLVARS_VERSION_DATE)

#define COLVARS_VERSION_UINT64(a,b,c) \
        ((UINT64_C(a) << 16) + (UINT64_C(b) << 8) + UINT64_C(c))
#define COLVARS_VERSION_UINT64_CURRENT \
        COLVARS_VERSION_UINT64(COLVARS_VERSION_YEAR, \
                               COLVARS_VERSION_MONTH, \
                               COLVARS_VERSION_DATE)

#endif

But it looks like that will break other shell scripts.

HanatoK avatar Jun 10 '25 19:06 HanatoK

I agree. If we make the changes you mention, the main scripts that will break are ours - if we adapt them, I doubt we'll break any other people's workflows, and if that happens, they will be developers who can just copy the logic from our updated version scripts.

By the way: nice job with the tutorial! I hadn't seen it. I think we should link to it from the Colvars documentation, because it's not very backend-specific (especially since you can build all codes with Tcl support :-)

jhenin avatar Jun 11 '25 13:06 jhenin

Same here on both points!

A compile-time comparable version number would be helpful to have, even if the new preprocessor code is slightly more complex than the current runtime code in colvarproxy::get_version_from_string().

If we adopt this, the function get_version_from_string() would have to be deleted, and it also would be nice if the Bash code in devel-tools/version_functions.sh was updated accordingly. (It's useful to avoid mistakes when manually updating colvars_version.h).

Lastly, the proxy class has its own version: we should either adopt the same scheme for it, or retire it altogether (we're not really using it).

giacomofiorin avatar Jun 13 '25 16:06 giacomofiorin

Same here on both points!

A compile-time comparable version number would be helpful to have, even if the new preprocessor code is slightly more complex than the current runtime code in colvarproxy::get_version_from_string().

If we adopt this, the function get_version_from_string() would have to be deleted, and it also would be nice if the Bash code in devel-tools/version_functions.sh was updated accordingly. (It's useful to avoid mistakes when manually updating colvars_version.h).

Lastly, the proxy class has its own version: we should either adopt the same scheme for it, or retire it altogether (we're not really using it).

I don't think get_version_from_string() has to be deleted. My proposal only changes colvars_version.h should be compatible with all the remaining code. However, the shell scripts relying on COLVARS_VERSION has to be changed.

HanatoK avatar Jun 13 '25 17:06 HanatoK

I don't think get_version_from_string() has to be deleted. My proposal only changes colvars_version.h should be compatible with all the remaining code. However, the shell scripts relying on COLVARS_VERSION has to be changed.

But you are changing the conversion from string to integer. If I build a test code using your header and the following lines:

  std::cout << cvm::main()->proxy->get_version_from_string(COLVARS_VERSION) << std::endl;
  std::cout << COLVARS_VERSION_UINT64_CURRENT << std::endl;

it gives:

20250512
132711692

giacomofiorin avatar Jun 13 '25 19:06 giacomofiorin

But you are changing the conversion from string to integer. If I build a test code using your header and the following lines:

std::cout << cvm::main()->proxy->get_version_from_string(COLVARS_VERSION) << std::endl; std::cout << COLVARS_VERSION_UINT64_CURRENT << std::endl; it gives:

20250512
132711692

You can have both if you want. cvm::main()->proxy->get_version_from_string(COLVARS_VERSION) can be used for runtime checking while leaving COLVARS_VERSION_UINT64_CURRENT for compile-time checking.

HanatoK avatar Jun 13 '25 19:06 HanatoK