Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Anisotropic rotational diffusion test #1622

Closed

Conversation

bogdan-tanygin
Copy link
Contributor

@bogdan-tanygin bogdan-tanygin commented Nov 14, 2017

Description of changes:

  • I've added the anisotropic rotational diffusion test. It is similar to a translational one which we already have in the mass-and-rinertia_per_particle.py. I've taken some measures to avoid a performance impact: limited loops for this specific calc and only 1 test case. Compare to overall bunch of python tests run, probably, there is not significant increase now: 17 minutes vs 15 minutes before. However, of course, up to the team to decide whether it is worth that. In my opinion: the rotational diffusion is a critically important physical process in Langevin Dynamics and its variance should be validated.
  • Refactoring in the lab<->body frame of reference transformation: now it is tests common based script.

PR Checklist

  • Tests?
    • Interface
    • Core
  • Docs?

@bogdan-tanygin
Copy link
Contributor Author

Maybe, it makes sense to have some tests (like this one) commented out or removed from the file testsuite/CMakeLists.txt. This test is performance consuming. However, the existing rotational motion algorithm is well developed and barely will fail some tests. Such tests are probably needed for some long period validations during significant MD changes. Particularly, I've planned to use it to show that the new Brownian Dynamics passes the same tests as existing LD. This is why I've opened this pull request (in a test driven way).

@bogdan-tanygin
Copy link
Contributor Author

This is one of tests which validate both Velocity Verlet LD and future BD. Hence, it indirectly relates to #1360.

@bogdan-tanygin
Copy link
Contributor Author

It is already a part of #1842.
I've not closed it only because I was not sure about the future of #1842. If it is already approved for the release 4.1 then we can close this PR.

@RudolfWeeber
Copy link
Contributor

The mass and rinertia test reached a complexity so that I don't really understand it anymore.
Could you please clarify in which way this pr goes beyond what is already tested in the Langevin thermostat testcase?
That tests roational and translatioal diffusion coefficients with global and per particle gamma and temperature via Green-Kubo and also it tests Maxwell-Boltzmann velocity distributions.

For rotation, only isotropic inertia tensors are tested.
Does this test go beyond that?

@bogdan-tanygin
Copy link
Contributor Author

@RudolfWeeber thanks for the feedback. I agree regarding the complexity, I already have the corresponding task #1905. The tests improvement here are tightly connected to the BD and LD results match comparison. I also need to backport the latest changes from the #1842.

Does this test go beyond that?

  1. Yes it does, this is an anisotropic rotational diffusion test. However, it requires too much of the test runtime. This is why I've relocated this part to the non-automically run test mass-and-rinertia_per_particle_rotdiff-longrun.py in the Brownian Dynamics within the existing VV loop #1842.

  2. Except the anisotropic rotational diffusion, by a physical nature, you are right. The Langevin thermostat testcase, indeed, validates fundamentally the same things. The mass and rinertia test does the same but as an integrated (/summed/averaged) practical result (variance = 2 * D * time).

Shortly, a relation is like this:

[langevin_thermostat.py] + [Es. VV integrator] + statistical rules = [mass-and-rinertia_per_particle]

If we are fully sure that the VV integrator is properly tested by other tests then, indeed, we could consider a removal of the test mass-and-rinertia_per_particle with a check that all test parameters are ported to the langevin_thermostat.

@bogdan-tanygin
Copy link
Contributor Author

This PR depends on #2021.

@bogdan-tanygin
Copy link
Contributor Author

For more durable runs I've detected the instability of the test. It could be either a technical or a conceptual issue. In the latter case extra theoretical/bibliography study is needed. Anyway, the present PR is not consistent anymore. I'll work on this separately (#2029).. Propose to close it.

@bogdan-tanygin bogdan-tanygin deleted the rot-diffusion-test branch December 25, 2018 18:05
kodiakhq bot added a commit that referenced this pull request Jan 25, 2020
Fixes #1360, also it contains the integrated changes from PRs #1622, #1757 which were required to validate both existing Langevin- (LD) and new Brownian Dynamics (BD) viscous and fluctuation dynamics including the rotational one.

Description of changes:
- Generally, this is a conventional simplest BD [schlick2010].
- Corresponding Sphinx documentation section is populated with the required details to reveal the concept.
- A velocity, even though it is optional for a simplest implementation of the BD, has been added for a consistency with the existing VV loop (c.f. #1360 (comment)) and all the existing tests and new tests. It does not impact the positional mechanics though which is natural for the BD [schlick2010].
- The velocity and position random walk has been added according to the classical diffusion equations [schlick2010,Pottier2014].
- Rotational motion BD is implemented analogously. Existing quaternion approach has been reused where possible.
- New test `brownian_thermostat.py` is based on the existing `langevin_thermostat.py`. It contains some commented out fragments which are inconsistent for the simplest BD cause ones correspond to more fine dynamical structure compare to the BD temporal relation `time_step >> mass / gamma`. The Maxwell distribution test is passing well and the Gaussian noise type is crucial for this which has been done in the `thermostat.hpp`.
- The `mass-and-rinertia_per_particle.py` contains changes described within the PRs #1622, #1757. Probably, it is of interest for both the LD and BD independently. This is why I've opened that PRs in advance.
- Existing automatic tests execution timeline is minimized.
- Additional "heavy" test has been added without an automatic run: `mass-and-rinertia_per_particle_rotdiff-longrun.py`. It requires ~2 hours of a run and provides the intensive validation of the existing LD and new BD dynamics in the same way.

PR Checklist
------------
 - [ ] Tests?
   - [ ] Interface
   - [ ] Core 
 - [ ] Docs?

_References_
schlick2010: https://link.springer.com/book/10.1007%2F978-1-4419-6351-2
Pottier2014: https://link.springer.com/article/10.1007/s10955-010-0114-6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants