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

Langevin docs update #1771

Merged
merged 8 commits into from
May 1, 2024
Merged

Conversation

crisbutu
Copy link
Contributor

Description

Made two updates to the documentation related to rotational motion with Langevin dynamics:

  1. redefined L as angular velocity instead of angular momentum
  2. changed the units of gamma_r from time^-1 to mass * length^2 * time^(-1)

Motivation and context

Resolves #1768

How has this been tested?

Manually made sure the latex changes parsed correctly by pasting them into an online latex editor.

Change log


Checklist:

@crisbutu crisbutu requested review from a team as code owners April 25, 2024 04:30
@crisbutu crisbutu requested review from SchoeniPhlippsn, tommy-waltmann and AlainKadar and removed request for a team April 25, 2024 04:30
Copy link
Contributor

@tommy-waltmann tommy-waltmann left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, we always like it when external folks contribute fixes!

One more thing for discussion: if $\vec L$ is actually the angular velocity, should we change the variable in the docstring to be $\vec \omega$ so its consistent with standard variable naming conventions?

hoomd/md/methods/methods.py Outdated Show resolved Hide resolved
hoomd/md/methods/methods.py Outdated Show resolved Hide resolved
crisbutu and others added 2 commits April 25, 2024 23:26
Co-authored-by: Tommy Waltmann <53307607+tommy-waltmann@users.noreply.github.com>
@crisbutu
Copy link
Contributor Author

@tommy-waltmann I addressed the comments and this is ready for another look!

@tommy-waltmann
Copy link
Contributor

pre-commit.ci autofix

@tommy-waltmann
Copy link
Contributor

This looks good, we now just need to satisfy the code and documentation style enforced by pre-commit. A list of lines that need fixing is included here: https://results.pre-commit.ci/run/github/147663007/1714140577.QTa6j2IcTPmDJO5n3xovrg .

@joaander
Copy link
Member

You can get fixes for:

hoomd/simulation.py:560:45: E226 missing whitespace around arithmetic operator
sphinx-doc/conf.py:63:21: E201 whitespace after '{'
sphinx-doc/conf.py:63:26: E202 whitespace before '}'

by merging the upstream trunk-patch into your branch.

@joaander joaander added the validate Execute long running validation tests on pull requests label Apr 26, 2024
@crisbutu
Copy link
Contributor Author

@tommy-waltmann pre-commit now passes

Copy link
Contributor

@tommy-waltmann tommy-waltmann left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@joaander joaander merged commit afbc231 into glotzerlab:trunk-patch May 1, 2024
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validate Execute long running validation tests on pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants