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

Add dpd contribution to pressure and pressure tensor #5045

Open
wants to merge 2 commits into
base: python
Choose a base branch
from

Conversation

mebrito
Copy link
Contributor

@mebrito mebrito commented Feb 18, 2025

Fixes #5030

Description of changes:

Comment on lines -495 to +499
return self._generate_summary(observable, 1, False)
observable = self._generate_summary(observable, 1, False)
if has_features("DPD"):
# Remove meaningless (key, value)
del observable["dpd"]
return observable
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be easier to make DPD stress part of the pressure calculation in the core, like in #5043? With the currently proposed changes, ESPResSo scripts written in C++ and Python disagree about the total pressure.

Copy link
Contributor Author

@mebrito mebrito Feb 19, 2025

Choose a reason for hiding this comment

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

I also thought on doing it in this alternative way, but since energy and pressure use the same class in the core for calculation, that would lead to an extra meaningless "dpd" key in the energy dictionary.

Copy link
Member

Choose a reason for hiding this comment

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

That would have been fine. Other features also have zero contribution in several fields, for example the cold Drude thermostat doesn't contribute to the energy.



@utx.skipIfMissingFeatures("DPD")
class DPDPressContrib(ut.TestCase):
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be easier to instead extend the existing DPD pressure test?

dpd_obs = espressomd.observables.DPDStress()
obs_stress = dpd_obs.calculate()
np.testing.assert_array_almost_equal(np.copy(dpd_stress), stress)
np.testing.assert_array_almost_equal(np.copy(obs_stress), stress)

One line to get the DPD pressure via system.analysis.pressure_tensor()["DPD"] and one line to check it's equal to -stress (pressure should be the negative of the stress).

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.

DPD pressure not part of total pressure
2 participants