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

Update documentation and clarify ambiguities #3673

Merged
merged 6 commits into from
Apr 20, 2020

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Apr 20, 2020

Description of changes:

  • remove CUDA installation instructions on OSX (follow-up to CI: Remove osx cuda job #3652)
  • update links to dockerfiles for non-Ubuntu OSes
  • correct the Coulomb prefactor formula (fixes Charge unit in espresso #3633)
  • in electrostatics scripts, replace magic values by their full expressions
  • document the difference between serial and parallel versions of libhdf5-dev (fixes H5md feature unavailable  #3656)
  • improve documentation of the script that shows the effect of MPI and node_grid on domain decomposition (see mailing list email "dividing simulation box using MPI" from 2020-04-20)

@jngrad jngrad added this to the Espresso 4.1.3 milestone Apr 20, 2020
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.


where

.. math::
C=\frac{1}{4\pi \varepsilon_0 \varepsilon_r}
C=\frac{e^2}{4\pi \varepsilon_0 \varepsilon_r}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe this to be correct. I don't think that any of this is related to the elementary charge (which is a constant of nature, one absolute amount of charge, whose numerical value depends on the unit system). I rather think that the charges are measured in whatever the charge units are.

Copy link
Member Author

Choose a reason for hiding this comment

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

We're missing an e^2 somewhere to make these equations consistent. The issue here is about definition. You probably think of the Coulomb prefactor C as the Coulomb constant k_e = 1/ (4 * pi * e0), which is also what I intuitively assumed the first time I read the text, but the text actually defines C with regards to the Bjerrum length.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well the interaction energy in espresso is C * q_1 * q_2 / r, independently of what the text defines...

Copy link
Member Author

Choose a reason for hiding this comment

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

We could rewrite the text, starting from the Coulomb equation U_C = k_e * q1 * q2 / r with k_e = 1/ (4 * pi * e0) and then introduce the ESPResSo convention where U_C = C * z1 * z2 / r with C defined from the Bjerrum length. Most ESPResSo users prefer to put an integer charge (aka charge number) on their coarse-grained particles and delegate the actual units to the prefactor C. This would clarify the ESPResSo convention while remaining in line with physics textbooks.

Copy link
Contributor

@fweik fweik Apr 20, 2020

Choose a reason for hiding this comment

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

I find it odd to push a unit system here, frankly. And charge numbers are not a concept that exists in electrodynamics. If you particles happen to not be ions, e is by no means a natural charge unit.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would do the former but this is also not a hill I'm willing to die on. I just find the idea that quantities have specific natural unit odd and mislead in general, and confusing in Espresso in particular because it does not have a fix unit system.

Copy link
Member Author

Choose a reason for hiding this comment

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

e is by no means a natural charge unit

This is a valid argument, how about introducing z_i as "normalized charges" z_i = q_i / e instead of formal charges?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not clear still on why they have to be introduced at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there da "definition" for C at all, when right after it says that is a settable parameter?
So I'd do the following. Drop the definition of C as such, then two examples which C to use for a relative permittivity, and for a specific Bjerrum length?

Copy link
Member Author

Choose a reason for hiding this comment

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

It would be simpler to just rewrite C=l_B k_B T / e^2 to fix the inconsistency. As you said above and in #3673 (comment), the user can move q around for convenience in simulations with ions, without changing the energy.

@codecov
Copy link

codecov bot commented Apr 20, 2020

Codecov Report

Merging #3673 into python will increase coverage by 0%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           python   #3673   +/-   ##
======================================
  Coverage      87%     87%           
======================================
  Files         533     533           
  Lines       22959   22959           
======================================
+ Hits        20156   20159    +3     
+ Misses       2803    2800    -3     
Impacted Files Coverage Δ
src/core/particle_data.cpp 96% <0%> (-1%) ⬇️
src/core/electrostatics_magnetostatics/p3m.cpp 85% <0%> (+<1%) ⬆️
src/core/polymer.cpp 98% <0%> (+5%) ⬆️

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 7f90057...135a717. Read the comment docs.

to color particles by node. With OpenMPI, this can be achieved using
``mpiexec -n 4 ./pypresso ../samples/visualization_cellsystem.py``.
Set property ``system.cell_system.node_grid = [n_x, n_y, n_z]``
(with ``n_x * n_y * n_z`` equal to the number of MPI threads) to
Copy link
Contributor

Choose a reason for hiding this comment

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

MPI threads shoudl be MPI processes or MPI ranks.

@fweik
Copy link
Contributor

fweik commented Apr 20, 2020

Sorry if this was a little bit confused, I think I can give a better explanation. Espresso implements an interaction of the form C * q_1 * q_2 / r, whose interaction strength is given by C. For a specific
value of C you can interpret this a electrostatics, e.g. if you derive this from a permittivity, it might
be the effective electrostatics in a medium. The Bjerrum length is just an odd/convenient way to state \epsilon_r, if you assume vacuum electrostatics of our universe (which Espresso does not force you to). C has units of [E] / ([L] * [Q]^2) so if you change the charge unit, the numbers for C and the charges changes, while the product stays the same. The energy is independent of the charge unit, as it should be.

Of course you can always move a charge factor from the q to C, but since you might be in some coarse grained world in your simulation, which does not deal in ions, but e.g. in dust particles, there
is no concept of a charge number and it is artificial to introduce it.

@jngrad
Copy link
Member Author

jngrad commented Apr 20, 2020

The energy is independent of the charge unit, as it should be.

Yes, the user just needs to be careful with dimensional analysis when setting up C and the charges. The tutorials and samples tweak the value of C to actually have an energy in kJ/mol, such that system.analysis.energy() gives an energy with that unit. As long as the same units are used in the WCA potential and thermostat, it should be fine.

Repositories offer serial and parallel versions of the libhdf5
package. Only the parallel version can be used by ESPResSo.
Copy link
Contributor

@fweik fweik left a comment

Choose a reason for hiding this comment

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

Sorry for the many words, I feel we could have gotten there quicke if I'd be more to the point ^^ LGTM now

@fweik fweik added the automerge Merge with kodiak label Apr 20, 2020
@kodiakhq kodiakhq bot merged commit f49d3b1 into espressomd:python Apr 20, 2020
@jngrad jngrad deleted the docs-4.2-p1 branch January 18, 2022 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

H5md feature unavailable Charge unit in espresso
3 participants