-
Notifications
You must be signed in to change notification settings - Fork 23
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
GMSH workflow #885
base: main
Are you sure you want to change the base?
GMSH workflow #885
Conversation
added GMSH workflow
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #885 +/- ##
=======================================
Coverage 99.56% 99.56%
=======================================
Files 61 61
Lines 2750 2750
=======================================
Hits 2738 2738
Misses 12 12 ☔ View full report in Codecov by Sentry. |
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 @celyneira for this great contribution! As you can see on this link the documentation didn't compile properly:
https://festim--885.org.readthedocs.build/en/885/userguide/mesh.html#meshes-from-gmsh
I've added a bunch of comments with corrections.
You can alos check that the docs compile correctly locally by following the Documentation Guide
docs/source/userguide/mesh.rst
Outdated
|
||
The Python API of GMSH can be used to create meshes that are useable in FESTIM. Here we will walk through its usage when creating a monoblock subsection consisting of tungsten surrounding a tube of CuCrZr | ||
|
||
<img width="948" alt="Screenshot 2024-09-11 at 17 09 10" src="https://github.com/user-attachments/assets/1ec52a67-840d-4ef8-a741-a79f0e2c1cc3"> |
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.
For images, please add them to the images folder and then add them to the documentation like this:
.. thumbnail:: ../images/path_to_your_image.png
:width: 400
:align: center
docs/source/userguide/mesh.rst
Outdated
``` | ||
import gmsh as gmsh | ||
|
||
gmsh.initialize() | ||
gmsh.model.add("mesh") | ||
``` |
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.
For code blocks, please use the same syntax as elsewhere in this file:
.. code-block:: python
your code goes here with an indent
This is not needed for inline code.
docs/source/userguide/mesh.rst
Outdated
gmsh.model.addPhysicalGroup(2, [7, 10], front_id, name = "front") | ||
gmsh.model.addPhysicalGroup(2, [6, 9], back_id, name = "back") | ||
gmsh.model.addPhysicalGroup(2, [1], left_id, name = "left") | ||
gmsh.model.addPhysicalGroup(2, [3], right_id, name = "right") | ||
gmsh.model.addPhysicalGroup(2, [4], top_id, name = "top") | ||
gmsh.model.addPhysicalGroup(2, [2], bottom_id, name = "bottom") | ||
gmsh.model.addPhysicalGroup(2, [5], outer_cylinder_surface_id, name = "outer_cylinder") | ||
gmsh.model.addPhysicalGroup(2, [8], inner_cylinder_surface_id, name = "inner_cylinder") | ||
|
||
gmsh.model.addPhysicalGroup(3,[1], tungsten_id, name = "tungsten") | ||
gmsh.model.addPhysicalGroup(3, [2], cucrzr_id, name = "cucrzr") |
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.
can we also tag the interfaces?
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 inner and outer cylinder surfaces are the interfaces but i've renamed them for clarity
docs/source/userguide/mesh.rst
Outdated
``` | ||
We have now created our mesh! | ||
|
||
However, for use in FESTIM, our mesh now has to be converted into XDMF files, and the surfaces and volume IDs extracted. |
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.
We could have a subsection heading here. The guide could be divided in two: 1) meshing the geometry 2) converting with meshio
docs/source/userguide/mesh.rst
Outdated
import festim as F | ||
|
||
model = F.Simulation() | ||
|
||
model.mesh = F.MeshFromXDMF(volume_file ="volume_mesh.xdmf", boundary_file = "surface_mesh.xdmf") | ||
|
||
model.materials = [F.Material(id=1, D_0=1, E_D=0), | ||
F.Material(id=2, D_0=5, E_D=0)] | ||
|
||
model.T = F.Temperature(800) | ||
|
||
model.boundary_conditions = [F.DirichletBC(surfaces = [top_id], value = 1, field = 0), | ||
F.DirichletBC(surfaces = [inner_cylinder_surface_id], value = 0, field = 0)] | ||
|
||
model.exports = [F.XDMFExport("solute")] | ||
|
||
model.settings = F.Settings( | ||
absolute_tolerance=1e-10, | ||
relative_tolerance=1e-10, | ||
transient=False, | ||
) | ||
|
||
model.initialise() | ||
model.run() | ||
|
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.
can you format this code using black please?
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 looks much better! I'll still need to run the tutorial to test things.
Could you format the python code blocks so that it conforms with black? Don't hesitate if you don't know how to do it
A few more comments
Meshes from GMSH | ||
---------------- |
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 can actually go replace the GMSH section instead
- Points | ||
- Lines | ||
- Wires / Curve Loops | ||
- whether we use curve loops or wires depends on whether we use the `.occ` or `.geo` geometry kernels. `.occ` allows for direct construction of more complex features such as cylinders, whereas using `.geo` requires explicit user definition of all the points, surfaces and volumes that would make up the cylinder. | ||
- Surfaces | ||
- Surface Loops | ||
- Volumes |
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.
In the docs this doesn't seem to compile correctly, maybe it needs a blank line before the first bullet
Meshing the geometry with GMSH | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
Firstly, GMSH must be imported and initialised. |
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.
Can we add a .. note::
on how to install GMSH?
|
||
A FESTIM simulation can then be run: | ||
|
||
.. code-block:: bash |
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.
.. code-block:: bash | |
.. code-block:: python |
added GMSH workflow
Proposed changes
Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.
Types of changes
What types of changes does your code introduce to FESTIM?
Checklist
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...