Document mode in md.pair.Pair #974 #1015

AlainKadar · 2021-05-25T20:08:38Z

Description

Added mode attribute to Pair base class. Also formatted mode attribute in each pair potential

Motivation and context

Mode attribute was missing in md.pair.Pair documentation. There were also inconsistencies in pair potentials documentation.

Resolves #974

How has this been tested?

Documentation was locally rendered in HTML.

Change log

Added mode attribute to documentation

Checklist:

I have reviewed the Contributor Guidelines.
I agree with the terms of the HOOMD-blue Contributor Agreement.
My name is on the list of contributors (sphinx-doc/credits.rst) in the pull request source branch.

… modes in pair potentials

AlainKadar · 2021-05-26T00:31:41Z

The Checks claim that pair.py was changed by the hook (i.e. white space was deleted) but I cannot see this deletion in the Files Changed. How are changes made by the hook reflected in the file on this PR/is there a further action I need to take?

vyasr · 2021-05-26T02:32:28Z

@AlainKadar Some of our other repos have pre-commit.ci enabled, which actually makes commits to open PRs when style fixes are needed. HOOMD doesn't currently have that enabled, so the style checks don't commit changes if any are necessary. The check you see changed the files locally when it ran, but those changes never got committed and were lost as soon as the Check ended. You need to make the same change and push, either manually (it looks like just deleting some whitespace on an otherwise empty line in pair.py) or by executing pre-commit run trailing-whitespace --file=hoomd/md/pair/pair.py.

joaander · 2021-05-26T12:21:58Z

It looks like the issue preventing pre-commit.ci from working on HOOMD is now fixed: pre-commit-ci/issues#53 . I will enable it and see if it works. It will push changes for all the fixers except for clang-format which we still need to run manually until pre-commit-ci/issues#63 is solved.

for more information, see https://pre-commit.ci

joaander

Thanks!

I have a few change requests.

hoomd/md/pair/pair.py

joaander · 2021-05-26T18:44:53Z

hoomd/md/pair/pair.py

+    Use the `params` dictionary to set
    potential coefficients. The coefficients must be set per unique pair of
    particle types.


Suggested change

Use the `params` dictionary to set

potential coefficients. The coefficients must be set per unique pair of

particle types.

Most of the potentials don't have this sentence any longer. Since we are updating them for consistency in this PR, let's remove this from all of them. The requirement is implicit in the definition of the TypeParameter type of params and it is covered in the tutorial.

for more information, see https://pre-commit.ci

AlainKadar

References/formatting fixed, thanks for the feedback.

Co-authored-by: Joshua A. Anderson <joaander@umich.edu>

joaander

Thanks!

bdice

Minor comments attached -- overall this is fine and I don't need to re-review.

bdice · 2021-05-28T23:17:15Z

hoomd/md/pair/pair.py

+        *mode*, *optional*: defaults to ``none``.
+        Possible values: ``none``, ``shift``, ``xplor``


String literal values should be wrapped in quotes. If this change is accepted, please also update the formatting in the docstring above these lines.

Suggested change

*mode*, *optional*: defaults to ``none``.

Possible values: ``none``, ``shift``, ``xplor``

*mode*, *optional*: defaults to ``"none"``.

Possible values: ``"none"``, ``"shift"``, ``"xplor"``

bdice · 2021-05-28T23:18:59Z

hoomd/md/pair/pair.py

@@ -323,7 +326,7 @@ class SLJ(Pair):
        nlist (`hoomd.md.nlist.NList`): Neighbor list
        r_cut (float): Default cutoff radius (in distance units).
        r_on (float): Default turn-on radius (in distance units).
-        mode (str): Energy shifting/smoothing mode
+        mode (str): Energy shifting mode


Some of these attributes end with periods and others do not. Try to make that consistent across all force parameter docstrings. Specifically I see periods missing from mode and nlist (nlist (`hoomd.md.nlist.NList`): Neighbor list).

Also it's unclear to me why the word "smoothing" was deleted here but kept in the description below.

Suggested change

mode (str): Energy shifting mode

mode (str): Energy shifting/smoothing mode.

bdice · 2021-05-28T23:21:00Z

hoomd/md/pair/pair.py

@@ -428,9 +429,7 @@ class Yukawa(Pair):
        \end{eqnarray*}

    See `Pair` for details on how forces are calculated and the available


Very minor nit, feel free to ignore this: the last word on this line, "available," is wrapped onto the next line in several of the Pair docstrings. Consistency might help future documentation writers that perform find/replace operations.

bdice · 2021-05-28T23:22:54Z

hoomd/md/pair/pair.py

@@ -635,9 +629,7 @@ class DPD(Pair):
    utilize the DPD functionality in your work.

    `DPD` does not implement and energy shift / smoothing modes due to the


I'm guessing this is supposed to say "any," but I suppose it might support the default behavior of "none"?

Suggested change

`DPD` does not implement and energy shift / smoothing modes due to the

`DPD` does not implement any energy shift / smoothing modes due to the

bdice · 2021-05-28T23:24:46Z

hoomd/md/pair/pair.py

@@ -718,9 +710,7 @@ class DPDConservative(Pair):


    `DPDConservative` does not implement and energy shift / smoothing modes due


Suggested change

`DPDConservative` does not implement and energy shift / smoothing modes due

`DPDConservative` does not implement any energy shift / smoothing modes due

bdice · 2021-05-28T23:26:47Z

hoomd/md/pair/pair.py

    of the dpd thermostat pair force with other integrators will result in
    unphysical behavior.

+    DPDLJ does not support xplor smoothing mode. See `Pair` for details on


Suggested change

DPDLJ does not support xplor smoothing mode. See `Pair` for details on

`DPDLJ` does not support smoothing with ``mode="xplor"``. See `Pair` for details on

bdice · 2021-05-28T23:27:18Z

hoomd/md/pair/pair.py

@@ -1454,7 +1435,7 @@ class LJ0804(Pair):
        nlist (`hoomd.md.nlist.NList`): Neighbor list
        r_cut (float): Default cutoff radius (in distance units).
        r_on (float): Default turn-on radius (in distance units).
-        mode (str): energy shifting/smoothing mode
+        mode (str): Energy shifting/smoothing mode


Suggested change

mode (str): Energy shifting/smoothing mode

mode (str): Energy shifting/smoothing mode.

bdice · 2021-05-28T23:27:22Z

hoomd/md/pair/pair.py

@@ -1398,7 +1380,7 @@ class LJ1208(Pair):
        nlist (`hoomd.md.nlist.NList`): Neighbor list
        r_cut (float): Default cutoff radius (in distance units).
        r_on (float): Default turn-on radius (in distance units).
-        mode (str): energy shifting/smoothing mode
+        mode (str): Energy shifting/smoothing mode


Suggested change

mode (str): Energy shifting/smoothing mode

mode (str): Energy shifting/smoothing mode.

bdice · 2021-05-28T23:27:26Z

hoomd/md/pair/pair.py

@@ -1343,7 +1326,7 @@ class Buckingham(Pair):
        nlist (`hoomd.md.nlist.NList`): Neighbor list
        r_cut (float): Default cutoff radius (in distance units).
        r_on (float): Default turn-on radius (in distance units).
-        mode (str): energy shifting/smoothing mode
+        mode (str): Energy shifting/smoothing mode


Suggested change

mode (str): Energy shifting/smoothing mode

mode (str): Energy shifting/smoothing mode.

bdice · 2021-05-28T23:27:29Z

hoomd/md/pair/pair.py

@@ -1174,7 +1158,7 @@ class ReactionField(Pair):
        nlist (`hoomd.md.nlist.NList`): Neighbor list
        r_cut (float): Default cutoff radius (in distance units).
        r_on (float): Default turn-on radius (in distance units).
-        mode (str): energy shifting/smoothing mode
+        mode (str): Energy shifting/smoothing mode


Suggested change

mode (str): Energy shifting/smoothing mode

mode (str): Energy shifting/smoothing mode.

AlainKadar added 2 commits May 25, 2021 12:13

Update pair docs to add mode attribute to pair base class and correct…

2756c44

… modes in pair potentials

Further updates for pair docs to include mode shifting

796c124

AlainKadar requested review from a team as code owners May 25, 2021 20:08

AlainKadar requested review from joaander and bdice and removed request for a team May 25, 2021 20:08

AlainKadar assigned gaofy95 and AlainKadar May 25, 2021

AlainKadar added 2 commits May 25, 2021 16:24

Break up lines which were too long

8d2d22f

delete whitespace

9e9cad8

joaander and others added 2 commits May 26, 2021 08:45

Merge branch 'master' into pair_docs

b06c608

[pre-commit.ci] auto fixes from pre-commit.com hooks

c87820a

for more information, see https://pre-commit.ci

joaander added the validate Execute long running validation tests on pull requests label May 26, 2021

Update credits

5bc08ad

joaander requested changes May 26, 2021

View reviewed changes

AlainKadar and others added 2 commits May 26, 2021 19:03

Remove references to params dictionary

1a89e96

[pre-commit.ci] auto fixes from pre-commit.com hooks

f51a48e

for more information, see https://pre-commit.ci

AlainKadar commented May 26, 2021

View reviewed changes

Fix incorrect formatting/references

fe9c84a

Co-authored-by: Joshua A. Anderson <joaander@umich.edu>

joaander approved these changes May 27, 2021

View reviewed changes

bdice approved these changes May 28, 2021

View reviewed changes

AlainKadar added 2 commits May 30, 2021 18:28

Formatting changes

543a53a

flake8 changes

4a5c2c8

joaander mentioned this pull request Jun 9, 2021

New potential with Mie parameters and SLJ parameters #1028

Merged

3 tasks

joaander merged commit 971e595 into master Jun 9, 2021

joaander deleted the pair_docs branch June 9, 2021 16:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document mode in md.pair.Pair #974 #1015

Document mode in md.pair.Pair #974 #1015

AlainKadar commented May 25, 2021

AlainKadar commented May 26, 2021

vyasr commented May 26, 2021 •

edited

joaander commented May 26, 2021

joaander left a comment

joaander May 26, 2021

AlainKadar left a comment

joaander left a comment

bdice left a comment

bdice May 28, 2021 •

edited

bdice May 28, 2021

bdice May 28, 2021

bdice May 28, 2021

bdice May 28, 2021

bdice May 28, 2021

bdice May 28, 2021

bdice May 28, 2021

bdice May 28, 2021

bdice May 28, 2021

	Use the `params` dictionary to set
	potential coefficients. The coefficients must be set per unique pair of
	particle types.

		mode, optional: defaults to ``none``.
		Possible values: ``none``, ``shift``, ``xplor``

	mode (str): Energy shifting mode
	mode (str): Energy shifting/smoothing mode.

		@@ -428,9 +429,7 @@ class Yukawa(Pair):
		\end{eqnarray*}

		See `Pair` for details on how forces are calculated and the available

		@@ -635,9 +629,7 @@ class DPD(Pair):
		utilize the DPD functionality in your work.

		`DPD` does not implement and energy shift / smoothing modes due to the

	`DPD` does not implement and energy shift / smoothing modes due to the
	`DPD` does not implement any energy shift / smoothing modes due to the

		@@ -718,9 +710,7 @@ class DPDConservative(Pair):


		`DPDConservative` does not implement and energy shift / smoothing modes due

	`DPDConservative` does not implement and energy shift / smoothing modes due
	`DPDConservative` does not implement any energy shift / smoothing modes due

	DPDLJ does not support xplor smoothing mode. See `Pair` for details on
	`DPDLJ` does not support smoothing with ``mode="xplor"``. See `Pair` for details on

Document mode in md.pair.Pair #974 #1015

Document mode in md.pair.Pair #974 #1015

Conversation

AlainKadar commented May 25, 2021

Description

Motivation and context

How has this been tested?

Change log

Checklist:

AlainKadar commented May 26, 2021

vyasr commented May 26, 2021 • edited

joaander commented May 26, 2021

joaander left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlainKadar left a comment

Choose a reason for hiding this comment

joaander left a comment

Choose a reason for hiding this comment

bdice left a comment

Choose a reason for hiding this comment

bdice May 28, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vyasr commented May 26, 2021 •

edited

bdice May 28, 2021 •

edited