Full rewrite of meshing for fractured domains#1576
Conversation
Merge work on gmsh object processing into main branch for new gmsh updates
New implementations, based on Gmsh OpenCASCADE to come
This is a helper module which should not be used externally
…ure functionality
Non-convex domains are no longer supported.
The tacit support for non-convex domains is hereby dropped
…fracture geometry
…://github.com/pmgbergen/porepy into 1477-methods-for-exporting-fracs-domains-gmsh
… and functionality
…d intersection test
Raise error messages if relevant.
Also maintenance and documentation
More systematic testing of possible scenarios. Also parametrization.
8af4b02 to
8f3feb9
Compare
Implementation must be cleaned up
8f3feb9 to
07444d6
Compare
|
View / edit / reply to this conversation on ReviewNB IvarStefansson commented on 2026-02-02T09:04:22Z Is the writing of all this logging default behaviour? I find it too verbose. keileg commented on 2026-02-02T11:39:49Z Should we make the gmsh verbosity level a user parameter? jwboth commented on 2026-02-02T12:22:31Z I don't know why, but I like it. So if at least there is a way of turning on info logging from gmsh, OK with me. |
|
View / edit / reply to this conversation on ReviewNB IvarStefansson commented on 2026-02-02T09:04:22Z Could we simplify to "It may be desirable to target the mesh resolution towards the vicinity of the fractures." ? I think the sentence in parentheses is more than important enough to warrant a promotion out of the parentheses! |
|
View / edit / reply to this conversation on ReviewNB IvarStefansson commented on 2026-02-02T09:04:23Z This seems to be a fundamental explanation that should come earlier. jwboth commented on 2026-02-02T12:45:24Z I agree. See my edits, that I keep for chronological understanding of my thoughts. This of course answers some of my questions. |
|
View / edit / reply to this conversation on ReviewNB IvarStefansson commented on 2026-02-02T09:04:24Z I think we should add a section the functionality for reading from file to manually tune grids and/or saving run time for complex geometries. keileg commented on 2026-02-02T11:38:57Z Agree, will do when merging the changes from that project into this PR. |
|
Agree, will do when merging the changes from that project into this PR. View entire conversation on ReviewNB |
|
Should we make the gmsh verbosity level a user parameter? View entire conversation on ReviewNB |
|
I don't know why, but I like it. So if at least there is a way of turning on info logging from gmsh, OK with me. View entire conversation on ReviewNB |
|
I agree. See my edits, that I keep for chronological understanding of my thoughts. This of course answers some of my questions. View entire conversation on ReviewNB |
|
View / edit / reply to this conversation on ReviewNB jwboth commented on 2026-02-02T12:50:12Z Use hyperlink to link to single phase flow tutorial.
It is somewhat unfortunate to communicate two ways of controlling meshing. Somewhat OK. But maybe consolidate and use the same in both tutorials? |
|
View / edit / reply to this conversation on ReviewNB jwboth commented on 2026-02-02T12:50:13Z I think this is better than before! Of course, it remains a black box what the effective cell size will be, but I am OK with that. My understanding of the concrete example is. With
Edit: It somehow does not surprise me based on my initial understanding, but maybe this is not a good example if the parameters are changed and the mesh looks the same. Better would be a visual support of the description of the target effect. |
|
View / edit / reply to this conversation on ReviewNB jwboth commented on 2026-02-02T12:50:14Z maybe add some extra interpretation? Something along the lines of aiming to control the mesh within the hull of the fracture network? Just to make sure that one targets here the general mesh and delaying the transition towards the
Edit: Looking at the subsequent plot, I am questioning what I just wrote. Which parameter was important for the control of the "hull size"? Based on the above edit, I am not sure whether it was the |
|
View / edit / reply to this conversation on ReviewNB jwboth commented on 2026-02-02T12:50:15Z As intersting for some applications as fracture propagation or fracture tip focus, maybe stress that it is neither possible to control the cell size close to fracture tips?
|
|
View / edit / reply to this conversation on ReviewNB jwboth commented on 2026-02-02T12:50:31Z Use hyperlink to link to single phase flow tutorial.
It is somewhat unfortunate to communicate two ways of controlling meshing. Somewhat OK. But maybe consolidate and use the same in both tutorials? |
|
View / edit / reply to this conversation on ReviewNB jwboth commented on 2026-02-02T12:50:32Z I think this is better than before! Of course, it remains a black box what the effective cell size will be, but I am OK with that. My understanding of the concrete example is. With
Edit: It somehow does not surprise me based on my initial understanding, but maybe this is not a good example if the parameters are changed and the mesh looks the same. Better would be a visual support of the description of the target effect. |
|
View / edit / reply to this conversation on ReviewNB jwboth commented on 2026-02-02T12:50:32Z maybe add some extra interpretation? Something along the lines of aiming to control the mesh within the hull of the fracture network? Just to make sure that one targets here the general mesh and delaying the transition towards the
Edit: Looking at the subsequent plot, I am questioning what I just wrote. Which parameter was important for the control of the "hull size"? Based on the above edit, I am not sure whether it was the |
|
View / edit / reply to this conversation on ReviewNB jwboth commented on 2026-02-02T12:50:33Z As intersting for some applications as fracture propagation or fracture tip focus, maybe stress that it is neither possible to control the cell size close to fracture tips?
|
IvarStefansson
left a comment
There was a problem hiding this comment.
Some effort! Only relatively minor comments, but quite a few of them. Let's discuss where indicated. Did not consider tests in this iteration.
| gmsh.initialize() | ||
|
|
||
| domain_tag = network.domain_to_gmsh() | ||
| fracture_tags = network.fractures_to_gmsh() |
There was a problem hiding this comment.
Am I being unreasonable if I claim this is somewhat sparsely documented?
| @@ -0,0 +1,150 @@ | |||
| """Contains classes representing two-dimensional fractures in 3D.""" | |||
There was a problem hiding this comment.
Is there a reason the class is ellipTIC and the file ellipSE?
| ) | ||
|
|
||
| def copy(self) -> Fracture: | ||
| """Return a copy of the fracture with the current vertices. |
There was a problem hiding this comment.
Is "current vertices" accurate?
| major_axis_angle=self.major_axis_angle, | ||
| strike_angle=self.strike_angle, | ||
| dip_angle=self.dip_angle, | ||
| index=self.index, |
There was a problem hiding this comment.
What does this imply about the interpretation of the index?
| order/sorting of vertices has to be implemented explicitly. | ||
| """ | ||
|
|
||
| def set_index(self, index: int) -> None: |
There was a problem hiding this comment.
See comment in ellipse_fracture.py. Should we specify intended use/interpretation?
|
|
||
| # For a boundary line to be an intersection, it must be shared by at least two | ||
| # fractures. | ||
| num_lines_occ = np.bincount(np.abs(bnd_lines).astype(int)) |
|
|
||
| ### Get hold of lines representing fractures and boundaries. | ||
| domain_entities = gmsh.model.get_entities(nd) | ||
| # TODO: If there is more than one domain entity (the domain is split into parts |
| # and assign the minimum mesh size among all occurrences of the point. | ||
| self._uniquify_mesh_size_dictionary(mesh_size_points) | ||
|
|
||
| # For lines that with no extra mesh size control points, assign an empty list. |
There was a problem hiding this comment.
| # For lines that with no extra mesh size control points, assign an empty list. | |
| # For lines with no extra mesh size control points, assign an empty list. |
| @@ -2721,6 +1147,9 @@ def to_csv(self, file_name: Path, domain: Optional[pp.Domain] = None) -> None: | |||
|
|
|||
| Domain specification. | |||
|
|
|||
| Raises: | |||
There was a problem hiding this comment.
I don't know if it is a priority, but we could create an issue about implementing it as exporting the parameters defining an elliptic fracture.
|
|
||
| """ | ||
| pts = self.pts.T | ||
| num_pts = len(pts) |
There was a problem hiding this comment.
| num_pts = len(pts) | |
| num_pts = pts.shape[0] |
IvarStefansson
left a comment
There was a problem hiding this comment.
Some effort! Only relatively minor comments, but quite a few of them. Let's discuss where indicated. Did not consider tests in this iteration.
Proposed changes
This PR is a major update of the functionality for meshing of fractured domains. The main changes are:
Beyond the module
fracs, the changes to the code base should be limited. Some tests have been updated, partly to ensure that the tests are not too computationally expensive.Migration to the new code
While the update should not be API breaking, the practical implications of the mesh size arguments will change. It is recommended to read the new meshing tutorial carefully before migrating to the new functionality.
Please not that fine tuning of the new algorithm may occur over the coming months.
Suggested workflow for review
These are my initial thoughts, they may or may not be good.
EDIT 04.02.26
This PR is downstream from #1573. The merging of that PR (when approved) into this branch will impact the following files:
fracture_importerand its tests are fully rewritten in 1573, there is no need to review the changes in this branch.See also todo list below.
What remains or have not been considered
EDIT: Todo list when merging in #1573
Types of changes
What types of changes does this PR introduce to PorePy?
Put an
xin the boxes that apply.Checklist
Put an
xin the boxes that apply or explain briefly why the box is not relevant.pytestwas run with the--run-skippedflag.