Add interface to the MiMiC framework for multiscale simulations#4546
Add interface to the MiMiC framework for multiscale simulations#4546oschuett merged 12 commits intocp2k:masterfrom
Conversation
|
Since we want to keep a linear git history, we don't allow merge commits. The usual tool for the job is
|
7a999d8 to
fb93ad7
Compare
fb93ad7 to
6fdb238
Compare
|
I've finally got to fix the pipeline and we're practically ready to release a compatible version of the MCL library. While I'll implement two last changes in the interface, I wanted to ask a few things:
Thanks for the help! Edit: Actually, I figured out 3 and 4. The changes will be added with the next commit, once I prepare a simple test system. |
tests/do_regtest.py
Outdated
| continue | ||
| batch = Batch(line, cfg) | ||
|
|
||
| # Read ENV.toml |
There was a problem hiding this comment.
I really don't want to introduce environment variables to CP2K. Adding a side-channel opens a can of worms. A calculation should be entirely specified by the input file.
So please add the keywords and sections you need to setup your tests.
Btw, I noticed that the mcl_log_1 files is pretty large. Any chance you could make it smaller?
Alternatively, we could also create a dedicated integration test like we did e.g. for i-PI.
There was a problem hiding this comment.
Sure. In that case I'll have a look at what has been done for i-PI since controlling it through input wouldn't do it for me, as these variables are required at the very beginning of a run, before the input is actually read. An alternative solution on my side would be to compile MCL in a way that would define these values and which would serve solely for testing purposes.
Regarding the file size, I was trying to make it as small as possible, but because it contains electron density and external potential values on the grid, this is how far I managed to get. I can either make the finest grid even coarser or I'll try dropping some data that is not actually checked.
EDIT: If we'd go with a standalone test, with removing a few large chunks of data and a possible compression, I think I can get below 1MB, although I'm not sure how much below.
There was a problem hiding this comment.
So, I managed to reduce the log file size to ~500 kB by removing the storage of data that weren't necessary for proper test run and also reducing the cutoffs and box size. The tested energy values are now far from being converged and possibly even unphysical, but I suppose it shouldn't matter too much for the test.
However, I'm still not sure what to do about the environment variable controls, since this is how the communication mechanism of the MCL library is controlled (see README) and unfortunately, it cannot be introduced in the CP2K input as these settings are already required right after the MPI
initialization.
The question is: would you like to refrain from introducing envvar support in do_regtest.py, because it might give an impression that this is a valid way of controlling CP2K, even though it is only used for external libraries? In that case, would it be okay if I implemented a specific type of a reg test in the script, or should we go for a standalone test? If it is the latter, I'm not sure how to approach this as it requires a specific build of CP2K (with MiMiC support), unlike the test for i-PI, in which i-PY is fairly independent (if I understand this correctly).
There was a problem hiding this comment.
would you like to refrain from introducing envvar support in do_regtest.py, because it might give an impression that this is a valid way of controlling CP2K,
Yes exactly!
So, how about we sweep it under the rug here:
Line 317 in 633ab39
There was a problem hiding this comment.
I hope that the current solution will do. Apart from this, could please check the modifications in message_passing.F? Should write a wrapper around the MCL routines there, or can it stay like it is?
There was a problem hiding this comment.
Since message_passing.F is already littered with #ifdefs, I don't think a wraper for MCL will help.
So, let's leave it as is for now.
We're planing to migrate away from the toolchain in the next year. So, it would be best to add MiMiC to Spack.
Yes, I'll remove the Makefile in a few days. |
20c7836 to
abeb7b1
Compare
|
We have finally managed to release a new version of MCL (communication library for MiMiC), but we don't have enough time to prepare a Spack package, so for the time being, I included it in the toolchain. Once ready, I'll change this. I now need to check, that the MiMiC interface test runs properly, but for that I need to run a parallel test (with 4 MPI processes). Could you please turn it on @oschuett ? |
|
Okay, from my side these are all the changes I wanted to make. Regarding that one failing test, I'm not really sure what's going on. For sdbg, it runs without any issues and when I run it locally with 4 MPI procs, it finishes with the correct value as well. If you have any ideas, please let me know, but I guess I'll be able to have a look at this next week. In the meantime, happy holidays! |
Yes, I also think this is ready to be merged. Unfortunately, there is now a tiny conflict that needs to be resolved first.
Don't worry. It's most likely caused by #4620. |
199a4ff to
1e7537f
Compare
1e7537f to
f189a02
Compare
Introduces the interface to MiMiC, a framework for multiscale simulations, currently supporting an electrostatic embedding QM/MM model. The interface is realized through its communication library, MCL.
Open tasks:
mimicmodule, except forMCL_Initialize(), this was to avoid dependency in thempiwrapmodule, but it probably isn't a correct solution.MPI_Abort()withMCL_Abort(), when CP2K is compiled with MiMiC support