Improvements to file generation methods, file templates, and a new universal templater function#48
Conversation
|
I'm also going to @PeterCannon10 - I think this is a bit much for your first PR, but since you'll be working with Ghastly, I'm pinging you so you can take a look, if you would like. |
Peter wants to re-review -- accidental approve button mash.
|
No worries - I'm also going to tag Megan, once I figure out her username. I'll require an approval from a more senior ARFC member before merging, so don't stress too much about having your review be perfect. If you end up only asking questions about how things work, that's fine by me. You can also always come in and ask questions in person, or in the undergrad slack. |
|
Alright - I made a few tweaks to the templates themselves to make restarting sims much easier. This didn't change anything in the python side of things. These should be good for review. I'd like to get this pulled in before I present on 2/19 (Thursday evening). |
| "zmax" : 0.0, | ||
| "zmin" : -0.1, | ||
| "r" : 0.1, | ||
| "open_bottom" : "", |
There was a problem hiding this comment.
Why is open_bottom set as the empty string for the outlet of both the cylinder and cone input files when the other aspects of the geometry have open_bottom set to "open 1"?
There was a problem hiding this comment.
I'm not seeing what you mean about a cone in this file, since the outlet is only a cylinder. But I can answer about the cylinder.
In LAMMPS, a region (such as a cone or cylinder) is assumed to be solid on all sides. If you want a particular side to allow particles to pass through it (so pebbles can flow) then you have to specify that when you define the region. The syntax for that is "open X', where X corresponds to which surface you want open (the numbers are defined in the LAMMPS documentation). For a cylinder, X=1 for the bottom, and X=2 for the top. Putting an empty string puts nothing, and leaves that surface solid.
The reason this particular surface is solid is because it's the bottom of the outlet, and I don't want the pebbles to fall out.
| settle = "" | ||
| case _: | ||
| _write_settle_block("settle.txt", sim_block, reg_names) | ||
| settle = "include settle.txt" |
There was a problem hiding this comment.
What is there a large gap in settle = "include settle.txt"?
There was a problem hiding this comment.
The gap is just to make sure the text matches the indentation level in the rest of the LAMMPS file - it's a style/readability thing, but it doesn't do anything beyond that.
LukeSeifert
left a comment
There was a problem hiding this comment.
Overall this looks good, the new template files clean things up a lot. I had a few comments on things, but generally I think there's only a few things that need minor changes.
|
I might make more small fixes here and there in the templates - I'll let you all know if it's a change to something other than the static portions of the templates. |
…pebbles if the region is not overfilled
Co-authored-by: LukeSeifert <44068471+LukeSeifert@users.noreply.github.com>
LukeSeifert
left a comment
There was a problem hiding this comment.
The changes look good! There are still one or two comments that you haven't responded to yet, but I think this will be good to merge once you take a look at those and make an issue or two on some of the other things that don't need to be taken care of in this PR (like adding more documentation, naming conventions, restructuring the _pack_core crp argument).
Summary of changes
This PR is an improvement to the auto-generated file templates and templating methods used in Ghastly. It also sets up the template directory to be better organized by organizing templates for LAMMPS into a new lammps/ directory inside the templates folder.
I added a _templater() function, which renders a file using the provided dict and template. While updating the functions to use the generic _templater, I found some opportunities to simplify file generation, and as a result, some functions were no longer needed, and have been removed.
Types of changes
Associated Issues and PRs
Checklist for Reviewers
Reviewers should use this link to get to the
Review Checklist before they begin their review.