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

Electrostatic pressure consistency #2409

Merged

Conversation

jonaslandsgesell
Copy link
Member

Fixes #2405

import espressomd
from espressomd import electrostatics
import tests_common
import numpy.testing as npt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move the external imports to the ones above

num_samples = 100
pressure_via_volume_scaling = np.nan
for i in range(num_samples):
self.system.integrator.run(100)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can use less integration steps?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will have an impact on the accuracy. it is a statistical equivalence of the pressures only.



if __name__ == "__main__":
print("Features: ", espressomd.features())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove print statement


import espressomd
from espressomd import electrostatics
import tests_common
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are not using tests_common

def test_p3m_pressure(self):
pressures_via_virial = []
pressures_via_volume_scaling = []
p3m = electrostatics.P3M(prefactor=2.0, accuracy=1e-3)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's a good idea to also check the GPU version?

@jonaslandsgesell
Copy link
Member Author

jonaslandsgesell commented Dec 7, 2018 via email

@KaiSzuttor
Copy link
Member

ValueError: The seed has to be an integer or a list of integers with one integer per node

@jonaslandsgesell
Copy link
Member Author

Please squash the commits as you merge.

@codecov
Copy link

codecov bot commented Dec 10, 2018

Codecov Report

Merging #2409 into python will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           python   #2409   +/-   ##
======================================
  Coverage      72%     72%           
======================================
  Files         396     396           
  Lines       18631   18631           
======================================
  Hits        13454   13454           
  Misses       5177    5177

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87fd43c...2820cb3. Read the comment docs.

Copy link
Member

@KaiSzuttor KaiSzuttor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a docstring to the test, so we know what is being tested and how without reading the code?

@fweik fweik merged commit 8a3c24e into espressomd:python Dec 20, 2018
RudolfWeeber pushed a commit to RudolfWeeber/espresso that referenced this pull request Jan 7, 2019
* add check

* add check

* patch

* imports

* up

* Update p3m_electrostatic_pressure.py

* format

* Update p3m_electrostatic_pressure.py

* apply patch

* exchange assert code

* docstring for test

* apply patch

* reformulate comparison

* year

* patch

* revert to np testing

* changed comparison to maximal 2 percent deviation

* changed comparison to maximal 2 percent deviation

* patch

* empty

* add tune skin

* add warmup

* patch

* refactor pressure calculation in small python class

* patch

* new style class

* add LJ to required features

* patch
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.

3 participants