Docs warning for charge in DielectricConstant #4263

PicoCentauri · 2023-08-28T18:51:53Z

Add a warning box to the documentation that only charge neutral systems can be handled by the current implementation.
See also #4249 for details on why this is important.

PR Checklist

Tests (we have a test for the error raise already.)?
Docs?
CHANGELOG updated?
Issue raised/referenced?

Developers certificate of origin

I certify that this contribution is covered by the LGPLv2.1+ license as defined in our LICENSE and adheres to the Developer Certificate of Origin.

📚 Documentation preview 📚: https://mdanalysis--4263.org.readthedocs.build/en/4263/

github-actions · 2023-08-28T18:54:31Z

Linter Bot Results:

Hi @PicoCentauri! Thanks for making this PR. We linted your code and found the following:

There are currently no issues detected! 🎉

codecov · 2023-08-28T19:12:13Z

Codecov Report

Patch coverage has no change and project coverage change: +0.01% 🎉

Comparison is base (e3354d1) 93.39% compared to head (a3d0bc6) 93.41%.

❗ Current head a3d0bc6 differs from pull request most recent head 1e5534c. Consider uploading reports for the commit 1e5534c to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4263      +/-   ##
===========================================
+ Coverage    93.39%   93.41%   +0.01%     
===========================================
  Files          170      183      +13     
  Lines        22224    23309    +1085     
  Branches      4065     4064       -1     
===========================================
+ Hits         20757    21775    +1018     
- Misses         951     1018      +67     
  Partials       516      516

Files Changed	Coverage Δ
package/MDAnalysis/analysis/dielectric.py	`100.00% <ø> (ø)`

... and 19 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

IAlibay · 2023-08-28T19:16:01Z

package/MDAnalysis/analysis/dielectric.py

@@ -65,6 +65,10 @@ class DielectricConstant(AnalysisBase):
    the usual case if electrostatics are handled with a Ewald summation
    technique. See [Neumann1983]_ for details on the derivation.

+    .. warning::
+      Applying this class requires that no free charges, such as ions or


Maybe a stupid question, if this is relying on an Ewald style summation, how well does it handle a system with a net charge?

No, totally valid question. The answer will be a bit longer. sorry.

Ewald can handle systems with a net charge. You introduce a homogenous background charge that neutralizes the system, which will lead to an additional force an all atoms. This is unproblematic for homogenous systems but lead to severe artifacts in inhomogenous systems like membranes etc.

The equation we use here is derived for an Ewald boundary condition, meaning that the dielectric constant of sorrounding medium is a metal ($\varepsilon=\infty$). If you have other boundary conditions like when using reaction field or cutoff electrostatics the equation will change as described in the paper we cite. We use eq 26.4.3D because the derivation for an Ewald electrostatic leads to the same results as a reaction field with an dielectric constant of $\infty$.

Now, why are charges a problem? The issue is that in the derivation you demand that Maxwell's equation for electric field

$$ \nabla \vec E = \frac{\rho}{\varepsilon_0} $$

where $\rho$ is the charge density is equal to zero: $\nabla \vec E = 0$. This means that no free charges are present. If you do not do this you will not end up with the equation we use.

Does this answer your question?

Do you think you could transfer this very insightful piece of writing to an issue for future reference. :)

Yes we sure. If people find it there.

hmacdope · 2023-08-29T00:05:24Z

package/MDAnalysis/analysis/dielectric.py

@@ -65,6 +65,10 @@ class DielectricConstant(AnalysisBase):
    the usual case if electrostatics are handled with a Ewald summation
    technique. See [Neumann1983]_ for details on the derivation.

+    .. warning::
+      Applying this class requires that no free charges, such as ions or


Do you think you could transfer this very insightful piece of writing to an issue for future reference. :)

hmacdope · 2023-08-29T00:06:49Z

package/MDAnalysis/analysis/dielectric.py

@@ -65,6 +65,10 @@ class DielectricConstant(AnalysisBase):
    the usual case if electrostatics are handled with a Ewald summation
    technique. See [Neumann1983]_ for details on the derivation.

+    .. warning::
+      Applying this class requires that no free charges, such as ions or
+      charged fragments, are present in the simulation.


@PicoCentauri perhaps I am misunderstanding but you can have no charged anything or is it that you need overall neutrality. Apologoies if silly q.

Nono it is fine. This is really a bit tricky. There are even publications where people do this wrong.

As we do it in the assertion in the class' prepare function you need charge neutral molecules. In a classical MD world they can have of course have partial charges. In fact, they have to because otherwise the system's total dipole moment would be zero and with it the dielectric constant.

What you can not have are ions such as Na+ Cl- or other charged fragments i.e negative carboxylic acid groups COO- or positive amine groups H3N+.

hmacdope

Im happy with the warning as is, once we resolve conflicts I think we can merge and address better documentation in #4262.

IAlibay

LGTM. Apologies I jumped in here and fixed the changelog conflicts. (no need to wait for CI again, this is a docs change anyways)

github-actions bot added the Component-Analysis label Aug 28, 2023

PicoCentauri force-pushed the dielectric_warn branch from fbf1c5a to f266906 Compare August 28, 2023 18:53

IAlibay reviewed Aug 28, 2023

View reviewed changes

Docs warning for charge in DielectricConstant

a3d0bc6

PicoCentauri force-pushed the dielectric_warn branch from f266906 to a3d0bc6 Compare August 28, 2023 19:39

hmacdope reviewed Aug 29, 2023

View reviewed changes

Update CHANGELOG

629ba9a

hmacdope approved these changes Sep 1, 2023

View reviewed changes

IAlibay added 2 commits September 2, 2023 12:03

Merge branch 'develop' into dielectric_warn

689cb5e

Update CHANGELOG

1e5534c

IAlibay approved these changes Sep 2, 2023

View reviewed changes

IAlibay merged commit c2e6df9 into MDAnalysis:develop Sep 2, 2023

IAlibay added the Component-Docs label Sep 21, 2023

PicoCentauri deleted the dielectric_warn branch June 5, 2024 15:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docs warning for charge in DielectricConstant #4263

Docs warning for charge in DielectricConstant #4263

PicoCentauri commented Aug 28, 2023 •

edited by github-actions bot

Loading

github-actions bot commented Aug 28, 2023 •

edited

Loading

codecov bot commented Aug 28, 2023 •

edited

Loading

IAlibay Aug 28, 2023

PicoCentauri Aug 28, 2023

hmacdope Aug 29, 2023

PicoCentauri Aug 29, 2023

hmacdope Aug 29, 2023

hmacdope Aug 29, 2023

PicoCentauri Aug 29, 2023 •

edited

Loading

hmacdope left a comment

IAlibay left a comment •

edited

Loading

Docs warning for charge in DielectricConstant #4263

Docs warning for charge in DielectricConstant #4263

Conversation

PicoCentauri commented Aug 28, 2023 • edited by github-actions bot Loading

PR Checklist

Developers certificate of origin

github-actions bot commented Aug 28, 2023 • edited Loading

Linter Bot Results:

codecov bot commented Aug 28, 2023 • edited Loading

Codecov Report

IAlibay Aug 28, 2023

Choose a reason for hiding this comment

PicoCentauri Aug 28, 2023

Choose a reason for hiding this comment

hmacdope Aug 29, 2023

Choose a reason for hiding this comment

PicoCentauri Aug 29, 2023

Choose a reason for hiding this comment

hmacdope Aug 29, 2023

Choose a reason for hiding this comment

hmacdope Aug 29, 2023

Choose a reason for hiding this comment

PicoCentauri Aug 29, 2023 • edited Loading

Choose a reason for hiding this comment

hmacdope left a comment

Choose a reason for hiding this comment

IAlibay left a comment • edited Loading

Choose a reason for hiding this comment

PicoCentauri commented Aug 28, 2023 •

edited by github-actions bot

Loading

github-actions bot commented Aug 28, 2023 •

edited

Loading

codecov bot commented Aug 28, 2023 •

edited

Loading

PicoCentauri Aug 29, 2023 •

edited

Loading

IAlibay left a comment •

edited

Loading