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

Barriers #111

Merged
merged 5 commits into from
Nov 21, 2023
Merged

Barriers #111

merged 5 commits into from
Nov 21, 2023

Conversation

joaander
Copy link
Member

Description

Add tutorial on barriers.

Resolves: #88

Checklist:

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@joaander joaander marked this pull request as ready for review November 14, 2023 16:31
@joaander joaander requested review from a team, SchoeniPhlippsn, tommy-waltmann and AlainKadar and removed request for a team November 14, 2023 16:31
08-Placing-Barriers/00-index.ipynb Outdated Show resolved Hide resolved
08-Placing-Barriers/01-Barriers.ipynb Outdated Show resolved Hide resolved
08-Placing-Barriers/01-Barriers.ipynb Outdated Show resolved Hide resolved
08-Placing-Barriers/02-Fixed-particles.ipynb Outdated Show resolved Hide resolved
08-Placing-Barriers/03-Wall-geometries.ipynb Outdated Show resolved Hide resolved
@tommy-waltmann
Copy link

The concept map powerpoint file appears to also be a part of this PR, I'm not sure if that was intentional or not.

@@ -0,0 +1,223 @@
{
Copy link

@syjlee syjlee Nov 14, 2023

Choose a reason for hiding this comment

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

Does it mean sphere with inside=False is not periodic across the box, but only cylindrical and planar wall are expected to be periodic(infinite)? If particles are in the non primary box image, sphere walls are not visible anymore?


Reply via ReviewNB

Copy link
Member Author

Choose a reason for hiding this comment

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

Particles are never outside the primary box image in HOOMD.

Spheres are not infinite surfaces by definition. If you were to place a sphere with a radius larger than box_L/2, then there would be discontinuities for particles near the edge of the box. Consider a radius box_L/2 + 0.1. The particle right on the box edge will still interact with the portion of the sphere just outside the box on the right edge - which would prevent it from ever exiting the primary box through the "gap".

@joaander
Copy link
Member Author

The concept map powerpoint file appears to also be a part of this PR, I'm not sure if that was intentional or not.

Yes, this is intentional as documented in CONTRIBUTING.md.

Copy link

@syjlee syjlee left a comment

Choose a reason for hiding this comment

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

I love this tutorial. Sections are well-coordinated and each section is visual/straightforward. Thank you Josh.

@joaander joaander merged commit 6646a73 into trunk Nov 21, 2023
3 checks passed
@joaander joaander deleted the barriers branch November 21, 2023 20:48
@@ -0,0 +1,223 @@
{
Copy link

@kmjens kmjens Jan 9, 2024

Choose a reason for hiding this comment

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

'BoxReisize' = typo. Correct to 'BoxResize'.

Also, "... will scale the periodic box and the particle positions but leave all..." should not have a comma before "but" because that is a comma splice; there is no subject in the second clause.


Reply via ReviewNB

@@ -0,0 +1,408 @@
{
Copy link

@kmjens kmjens Jan 9, 2024

Choose a reason for hiding this comment

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

Very minor comment in the Questions section that it "molecular dynamics" is referred to by full name here but the acronym "HPMC" is used in the next section. Unimportant, but I still noticed it was inconsistent.


Reply via ReviewNB

@@ -0,0 +1,408 @@
{
Copy link

@kmjens kmjens Jan 9, 2024

Choose a reason for hiding this comment

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

Add the word "mobile":

".... place mobile particles in the center..."


Reply via ReviewNB

@@ -0,0 +1,408 @@
{
Copy link

@kmjens kmjens Jan 9, 2024

Choose a reason for hiding this comment

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

"in from the edge" is awk wording. Maybe just: "...inside the edge of the box..." or a different alternative.


Reply via ReviewNB

@@ -0,0 +1,349 @@
{
Copy link

@kmjens kmjens Jan 9, 2024

Choose a reason for hiding this comment

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

Which documentation? For the specific shape? Or the specific wall? Perhaps an inclusion of the link could be helpful on the word "documentation".

On the documentation it is written:

"See the individual subclasses of hoomd.hpmc.integrate.HPMCIntegrator for their wall support." which I find slightly more clear.

Also, this is the only use of the word "inactive" in the tutorial. Maybe find a consistent word to describe the side with and with out particles. "inaccessible" or "prohibited" may be more appropriate.


Reply via ReviewNB

@@ -0,0 +1,349 @@
{
Copy link

@kmjens kmjens Jan 9, 2024

Choose a reason for hiding this comment

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

"...the previous section showed how to run ..."


Reply via ReviewNB

@joaander
Copy link
Member Author

@kmjens Thanks for the review. I applied the fixes in f962d8d and f0ba716.

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.

Tutorial: Walls
4 participants