-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Mesh tutorial #155
base: trunk-minor
Are you sure you want to change the base?
Mesh tutorial #155
Conversation
Add Kate and Philipp to authors list
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
@@ -0,0 +1,701 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest moving these parameters down to the code blocks where they are used. At this point in the tutorial, the user has no idea what the parameters mean or what they are used for.
Reply via ReviewNB
@@ -0,0 +1,701 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a markdown block that says something to the effect of "Let's run a simulation with a spherical mesh. As shown in the previous section, use the Fibonacci sphere to initialize the mesh."
Reply via ReviewNB
@@ -0,0 +1,701 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code also needs some explanation. It is not obvious to a new user that the vertices of the mesh will become particles in the simulation. I'm not sure if that full explanation belongs here. It may be better to put a whole paragraph at the beginning that outlines the plan to place the vertices as particles and apply potentials that are functions of the mesh.
Reply via ReviewNB
@@ -0,0 +1,701 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a simulation is not a new concept for readers, but initializing the mesh object is. I would condense the "standard" initialization into one code block and preface it with a short description.
Reply via ReviewNB
@@ -0,0 +1,701 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MD integration is also not new to users - you can probably merge it with the section above.
Reply via ReviewNB
@@ -0,0 +1,701 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what this code block is trying to convey. The tether parameter looks to belong with the "mesh_bond_potential" code below. The rest of the code looks like it belongs in its own block with some explanation of how these values might be used to validate the tether parameter choices?
Reply via ReviewNB
@@ -0,0 +1,701 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -0,0 +1,701 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rephrase the 2nd sentence: "Pass the mesh_obj
to the neighborlist and set the "meshbond"
exclusion to exclude WCA forces between vertices in the same triangle".
Also, does this really exclude all vertices in the same triangle? Or only those that form an edge?
Lastly, WCA has not previously been defined in the tutorials. You could replace "WCA" with "repulsive".
Reply via ReviewNB
@@ -0,0 +1,701 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rest of the sections need some explanation to describe what the potential is, how it affects the simulation, and when a user might want to add it (or not).
For a more visual guide, you might want to run the simulation without any rigidity potentials first to show the user the flexible mesh. Then turn on the rigidity potentials (possibly one at a time) to show the resulting behavior.
Reply via ReviewNB
@@ -0,0 +1,701 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the volume need to be slowly changed here? What if you just abruptly set the new volume and run the simulation? I think Brownian
would be able to absorb the energy, but maybe not. Taking out the custom action would make the tutorial easier for new users to understand.
Reply via ReviewNB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I pushed some spelling and formatting fixes directly to the branch. The first section of the tutorial is great, simple and to the point. I like the code examples in the second, but I think they could use a little more explanation and a little rearranging to more clearly teach the new concepts (detailed comments on each block in reviewNB).
Description
Add a mesh tutorial that describes how to create and include meshes in hoomd.
Checklist: