MRChemSoft / mrchem

MultiResolution Chemistry
GNU Lesser General Public License v3.0
30 stars 21 forks source link

Surface forces #488

Closed moritzgubler closed 4 days ago

moritzgubler commented 5 months ago

This is my implementation of the idea I had of calculating forces with surface integrals.

It works quite well and the forces are more accurate. Also, mrchem struggles to compute forces when the world precision is 1e-7 or smaller. Then it requires a lot of memory (more than 32 GB for a H2 molecule). This is not the case with my approach . Geometry optimizations seem to be reliable if the stopping criterion is chosen 10 * world_prec.

I have already put all the parameters into the parser, so it is quite simple to test and run my code (look for the section "- name: Forces" in the template.yaml input parser file).

It works with lda and gga functionals and for both closed and open shell systems. At the moment, there is one thing that might be improved in the future:

robertodr commented 5 months ago

hei Moritz, could you add a quick description of how this differs from the Hellmann-Feynman calculation of the forces? Also, I have C++ code that:

I can share both if it's of interest.

moritzgubler commented 5 months ago

I find i kind of usefule to be able to exchange the lebvedev file with something else and not have to recompile the code to test things. But if you like it more to avoid reading the file, I can also use your method.

What would be the purpose of the Leopardi points? are they more accurate than the lebvedev grids?

I sent you a small description of my method on zulip. I don't want to publish this yet.

robertodr commented 5 months ago

I find i kind of usefule to be able to exchange the lebvedev file with something else and not have to recompile the code to test things. But if you like it more to avoid reading the file, I can also use your method.

Yes, of course. The approach is the same, since the grids are known beforehand. If you tabulate all of them, there's not much you can swap them with, as their structure is not shared (that I know of) with other grids on the sphere. I dislike reading files at runtime. It's Lebedev, not lebvedev, by the way.

What would be the purpose of the Leopardi points? are they more accurate than the lebvedev grids?

In terms of accuracy they're similar. They're easier to generate though, so you're not limited to a pre-tabulated set of points and weights.

I sent you a small description of my method on zulip. I don't want to publish this yet.

Thanks!

moritzgubler commented 5 months ago

Can you send me the lebedev cpp library please you were talking about? I can also integrate that to avoid reading the file.

robertodr commented 5 months ago

here you go https://github.com/robertodr/balls

moritzgubler commented 4 months ago

Thanks, I integrated your library into my code.

moritzgubler commented 4 months ago

Open shell calculations now also work, I still need to implement the GGA stress density (see my question on zulip) and make sure, that the forces are calculated using the hellmann-feynman theorem in an exact exchange calculation.

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 2.54735% with 1492 lines in your changes missing coverage. Please review.

Project coverage is 62.59%. Comparing base (df39ee2) to head (13b89d8). Report is 2 commits behind head on master.

Files Patch % Lines
src/surface_forces/SurfaceForce.cpp 0.00% 220 Missing :warning:
src/surface_forces/xcStress.cpp 0.00% 125 Missing :warning:
src/surface_forces/detail/lebedev_utils.cpp 0.00% 107 Missing :warning:
src/surface_forces/LebedevData.cpp 0.00% 75 Missing :warning:
src/surface_forces/lebedev.cpp 0.00% 36 Missing :warning:
src/surface_forces/detail/Lebedev_021_0170.hpp 0.00% 33 Missing :warning:
src/surface_forces/detail/Lebedev_023_0194.hpp 0.00% 33 Missing :warning:
src/surface_forces/detail/Lebedev_027_0266.hpp 0.00% 33 Missing :warning:
src/surface_forces/detail/Lebedev_035_0434.hpp 0.00% 33 Missing :warning:
src/surface_forces/detail/Lebedev_047_0770.hpp 0.00% 33 Missing :warning:
... and 31 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #488 +/- ## ========================================== - Coverage 68.66% 62.59% -6.08% ========================================== Files 194 233 +39 Lines 15287 16790 +1503 ========================================== + Hits 10497 10509 +12 - Misses 4790 6281 +1491 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

moritzgubler commented 4 months ago

Hi @robertodr @ilfreddy I implemented now everything I wanted to, cleaned up to code and added a bit of documentation. The new method to calculate the forces now works for LDA and GGA functionals for both restricted and unrestricted calculations. The pulll request is now ready for review. Best, Moritz

moritzgubler commented 3 months ago

I previously tested GGA calculations only for H2 where the forces where the forces were correct. In more complicated systems this is not the case. (LDA is always correct). I will look into it and let you know once I have solved the problem.

moritzgubler commented 3 months ago

The GGA bug is fixed now, I forgot to add the divergence terms in the xc potential for the GGA case. They have been added now. MPI does not work anymore, it segfaults in the calculation of the kinetic stress. I suspect, that I try to access orbitals in an orbitalvector that are stored on another rank. @gitpeterwind Do you know what could be the problem?

moritzgubler commented 3 months ago

I can recreate the error with this input:

world_prec = 1.0e-4
world_unit = bohr

WaveFunction {
  method = blyp
  restricted = false
}

Properties {
  geometric_derivative = true
}
Forces {
    method = "surface_integrals"
    #method = 'hellmann_feynman'
    surface_integral_precision = medium
}

Molecule {
$coords
H 0.86387418 -0.02789966 0.13501906
Li -0.44463806 -0.04858572 -0.04959119
$end
}

I start the simulation with:

mrchem --dryrun hli.inp
mpirun -np 4 mrchem.x hli.json
ilfreddy commented 3 months ago

@moritzgubler would you mind adding also some testing to your code? According to the code coverage report, your patch is only marginally covered. Tests are essential to make sure the code does not get broken by others later on.

moritzgubler commented 3 months ago

I added a test, is there a way to look at the codecov report?

ilfreddy commented 3 weeks ago

This looks good to me now. Just a couple of minor questions.

@moritzgubler

  1. could you update the description at the top of the PR to reflect the actual state of the PR (it still says GGA is not included).
  2. a question: how about other functionals (like meta-GGA), Would they require additional implementation
  3. Is HF exchange working now?

@stigrj this branch includes a merge a few commits ago. Would you let it pass or should it be changed to a rebase?

moritzgubler commented 3 weeks ago

@ilfreddy

  1. lda, gga, closed and open shell are working, the description is updated.
  2. the meta-GGA contribution to the stress would have to be implemented. This should be quite easy. I thought only l and gga functionals are implemented in mrchem.
  3. No. This would require the exchange stress density. This paper might help: https://pubs.acs.org/doi/10.1021/acs.jctc.0c01194?ref=pdf
ilfreddy commented 3 weeks ago

@ilfreddy

  1. lda, gga, closed and open shell are working, the description is updated.
  2. the meta-GGA contribution to the stress would have to be implemented. This should be quite easy. I thought only l and gga functionals are implemented in mrchem.
  3. No. This would require the exchange stress density. This paper might help: https://pubs.acs.org/doi/10.1021/acs.jctc.0c01194?ref=pdf

Thanks for the reference! Indeed we only have GGA at present, but we have some plans to make a libXC interface and thus include also metaGGA functionals. But I guess at this point it might be more relevant to focus on HF exchange.