New features that need TensorFlow link want to contribute

yanze039 commented 3 years ago

Hi all, We(@amcadmus, @njzjz, @TablewareBox, @Dead-fisher) want to contribute our new features to PLUMED2, which can read TensorFlow graph files and use their outputs as biased force adding to the simulations. The problems come from the link library.

Currently, we install our modified PLUMED based on the following steps:

firstly we compile TensorFlow with C++ interface,
we add our self-compiled .cpp file to the plumed/src/bias directory.
when configuring PLUMED, we add gcc flags (e.g. ./configure --prefix=/software/plumed252 CXXFLAGS="-std=gnu++11 -I $tf_path/include/" LDFLAGS=" -L$tf_path/lib -ltensorflow_framework -ltensorflow_cc -Wl,-rpath,$tf_path/lib/") to let libtensorflow and head files be touched.

We want add this function to PLUMED2 officially. We would like to assume that the users have compiled TensorFlow with C++ interface by themselves and known where the library files are already. So that, when using this feature, PLUMED can find the corresponding head files and link library from the path that the users offer. Thus, what we want to do is to add our self-compiled file to PLUMED and get the support for dynamic link library (so that not necessary linking to TensorFlow when compiling PLUMED, only needed when calling this feature)

Is this recommended or any better suggestion given to us? Thanks!

For your information: Here is our Project(Reinforced Dynamics, RiD): deepmodeling/rid-kit Here is our .cpp file: DeepFE.cpp Our works have been published at:

Zhang, L., Wang, H., E, W.. Reinforced dynamics for enhanced sampling in large atomic and molecular systems[J]. The Journal of chemical physics, 2018, 148(12): 124113 Wang, D., Zhang, L., Wang, H., E, W.. Efficient sampling of high-dimensional free energy landscapes using adaptive reinforced dynamics[J]. arXiv preprint arXiv:2104.01620, 2021.

GiovanniBussi commented 3 years ago

@Dead-fisher thanks for your message. Please have a look at this as well.

What you need to do is:

Add a proper section to the configure.ac file and run autoconf. You can get inspiration from how we link boost libraries there. In the end, there should be a flag --enable-tensorflow that checks if TensorFlow is available on that system, and that by default is off. I would suggest writing a short C++ test, as we do for boost.
Protect all the code that needs TensorFlow headers with #ifdef __PLUMED_HAS_TENSORFLOW. Again, search the code for #__PLUMED_HAS_BOOST to see an example.
Put you code in an optional module (you can get inspiration from the existing ones), maybe called deepfe.
If you want your code in 2.8, please consider an approximate deadline at end of September / middle October, when we will release 2.8 beta.

Once this is done you should be able to configure with --enable-tensorflow --enable-modules=deepfe, plus obviously the required flags for setting the paths to TensorFlow

For us to include it we also require you to write a regression test. You can get inspiration on those available in the regtest directory. A good example is the drr module, that indeed requires boost. As you see here, in every test you should specify that this test can only be done if you have enabled deepfe and have TensorFlow installed.

As I wrote on the mailing list, the regtest should be runnable in our CI (currently on GitHub action). This would imply installing TensorFlow there at every build. We do it with boost and other libraries. I don't know how easy it is to do it with TensorFlow. A typical issue with C++ libraries is that in order to link them to a program compiled with debug flags they should use debug flags as well, that's why we explicitly compile boost when using debug flags.

It is important to realize that if we do not have a way to test your code in our CI workflow we will not be able to maintain it and keep it in sync with the rest of PLUMED. In addition, your feature will not appear in the manual (that is compiled based on which features are available in our CI workflow). So I personally consider this as a critical step for inclusion.

Finally, I don't understand this:

Thus, what we want to do is to add our self-compiled file to PLUMED and get the support for dynamic link library (so that not necessary linking to TensorFlow when compiling PLUMED, only needed when calling this feature)

This is not enabled for any of the modules currently distributed with PLUMED. I mean: modules (and dependent libraries) should be available at compile and link time. If you want to allow users to load your deepfe file at runtime, please disregard the instructions above and use the LOAD keyword. I can help you in solving issues with passing the proper paths to the LOAD keyword if necessary (see #477). But notice that in this way your code will not be included in the official PLUMED release.

yanze039 commented 3 years ago

Hi Bussi @GiovanniBussi , Thanks for your great suggestions and instructions. I have read all the websites you have mentioned. And I also encountered some problems when I am coding the configure.ac.

The first problem comes from this webpage: Using external libraries in PLUMED. It asks me to add PLUMED_CHECK_PACKAGE([header_file.hpp],[function],[__PLUMED_HAS_LIBRARY_NAME],[library_name]) to configure.ac file. but I am not clear which [function] I should choose here. For example, one of the header files is #include "tensorflow/core/public/session.h", so I write the function as: PLUMED_CHECK_PACKAGE([tensorflow/core/public/session.h], [some_function],[__PLUMED_HAS_TENSORFLOW],[tensorflow]). So, what kind of function should be added here?
The second problem is that, when I choose arbitrary one function, like NewSession, and then autoconf and configure with command ./configure --prefix=some_path --enable-tensorflow --enable-modules=deepfe CXXFLAGS="-std=gnu++11 -I $tf_path/include/" LDFLAGS=" -L$tf_path/lib -ltensorflow_framework -ltensorflow_cc -Wl,-rpath,$tf_path/lib/" , where $tf_path is a variable pre-defined. The log file told me that :
```
configure:4689: checking for tensorflow/core/public/session.h
configure:4689: result: no
configure:4818: WARNING: cannot enable __PLUMED_HAS_TENSORFLOW
```
Even if the path is correct, and session.h dose exist in the path $tf_path/include/tensorflow/core/public/session.h. So what is wrong?

Thanks!

GiovanniBussi commented 3 years ago

I think that documentation is kind of outdated (it refers to C libraries, but with C++ libraries sometime things to do not work, I don't know why). I would follow what we did in boost:

...
PLUMED_CONFIG_ENABLE([tensorflow],[search for tensorflow],[no])
...
if test $tensorflow == true ; then
  PLUMED_CHECK_CXX_PACKAGE([tensorflow],[
#include < tensorflow/core/public/session.h >
int
main ()
{
  auto a=NewSession();
// not sure about this, just write a little C++ code that should compile only if library is available
// make sure it is something that would only link correctly if tensorflow library is linked (so do not use
// class that is fully inlined!)
  return 0;
}
  ], [__PLUMED_HAS_TENSORFLOW])
fi
...

then

autoconf

then

./configure CPPFLAGS="-I $tf_path/include/" LDFLAGS="-L$tf_path/lib -ltensorflow_framework -ltensorflow_cc -Wl,-rpath,$tf_path/lib/"

yanze039 commented 3 years ago

Hi Bussi @GiovanniBussi ,

Thanks again for your reply. I change the function to PLUMED_CHECK_CXX_PACKAGE. Now the configure.ac looks quite like the demo you gave me. However, the it still reports errors like:

configure:4140: checking tensorflow without extra libs
configure:4157: gcc -o conftest -g -O2 -I /home/dongdong/wyz/software/gromacs-dp-rid-mod-summer/include/  -L/home/dongdong/wyz/software/gromacs-dp-rid-mod-summer/lib -ltensorflow_framework -ltensorflow_cc -Wl,-rpath,/home/dongdong/wyz/software/gromacs-dp-rid-mod-summer/lib/ conftest.c  >&5
In file included from conftest.c:10:0:
/home/dongdong/wyz/software/gromacs-dp-rid-mod-summer/include/tensorflow/core/public/session.h:19:18: fatal error: string: No such file or directory
compilation terminated.
configure:4157: $? = 1
configure: failed program was:
...
.....
configure:4162: result: no
configure:4178: WARNING: cannot enable __PLUMED_HAS_TENSORFLOW

I am confused that, I have wrapped my blocks by PLUMED_CHECK_CXX_PACKAGE, but configure still usesgcc ... conftest.c to check my demo codes. Also, all header files about tensorflow are written in C++ format ,like with #include <string>, which may lead to the errors above 'fatal error: string: No such file or directory'.

Is there something wrong with my operations? Thanks!

GiovanniBussi commented 3 years ago

That's not what happens when testing boost...

Are you sure you added the two snippets in the right place? Do it close to the corresponding boost_graph snippets. I am afraid autoconf macro language might do something wrong if you do not place them in a position where it knows we are using C++

yanze039 commented 3 years ago

Hi Bussi @GiovanniBussi,

It works! I passed the ./configure check now. The new problems is that, when I use make command, it will output:

make[6]: Leaving directory `/home/dongdong/wyz/plumed2/src/core'
make -C ../deepfe links
make[6]: Entering directory `/home/dongdong/wyz/plumed2/src/deepfe'
make[6]: *** No rule to make target `links'.  Stop.

I have no idea what's wrong. I have googled this errors, but those answers are not related to mine. Under my subdirectory, the files are:

── deepfe
│   ├── DeePFE.cpp
│   ├── MakeFile
│   └── module.type

MakeFile contains:

USE=core tools bias
# generic makefile
include ../maketools/make.module

Hope you can help me again!

GiovanniBussi commented 3 years ago

Target links: is defined in the file ../maketools/make.module, so this is an unexpected error.

Are you sure that MakeFile is ok? I think it should be named Makefile (lowercase F).

plumed / plumed2

New features that need TensorFlow link want to contribute #733