Mixed-dimensional grids for case 3, flow benchmark 3d

[jhabriel]

Proposed changes

This PR introduces the possibility to create mdgs for the geometry corresponding to case 3 of the 3d flow benchmark. Four level of refinements are available (levels 0 and 1 are the ones used by UiB in the benchmark). A weak in-situ test is incorporated in the function.

Please, feel free to modify and extend the functionality as you see fit.

Types of changes

What types of changes does this PR introduce to PorePy?
Put an x in the boxes that apply.

• Minor change (e.g., dependency bumps, broken links).
• Bugfix (non-breaking change which fixes an issue).
• New feature (non-breaking change which adds functionality).
• Breaking change (fix or feature that would cause existing functionality to not work as expected).
• Testing (contribution related to testing of existing or new functionality).
• Documentation (contribution related to adding, improving, or fixing documentation).
• Maintenance (e.g., improve logic and performance, remove obsolete code).
• Other:

Checklist

Put an x in the boxes that apply or explain briefly why the box is not relevant.

• The documentation is up-to-date.
• Static typing is included in the update.
• This PR does not duplicate existing functionality.
• The update is covered by the test suite (including tests added in the PR).
• If new skipped tests have been introduced in this PR, pytest was run with the --run-skipped flag.

[keileg]

Thanks @jhabriel. We will have a look.

[keileg]

I will add some testing of the new functionality.

[keileg]

@IvarStefansson: I added some testing and renamed the folder as we discussed. Could you please have a look at the changes that I introduced?

I have no further comments to the PR.

[IvarStefansson]

Only one substantial comment

[jhabriel]

@keileg: I think you forgot to add the CSV file to the subdirectory benchmark_3d_case_3. If you are copying and pasting from porepy_mesh_factory, please note that the first line (i.e., the domain specification) is not correctly defined. Instead of 0, 0, 0, 1, 1, 1 it has to be 0, 0, 0, 1, 2.25, 1.

Another point to consider is that we might want to retrieve only the fracture network. For these cases, creating the mdg will be quite time-consuming. Would you consider adding an only_network: bool = False input parameter to cover such cases?

Finally, this might be a rare case, but: How do we know that there is a 1-to-1 match between the fracture network and the mdg? Both, in terms of coordinates and indexing.

[keileg]

@keileg: I think you forgot to add the CSV file to the subdirectory benchmark_3d_case_3. If you are copying and pasting from porepy_mesh_factory, please note that the first line (i.e., the domain specification) is not correctly defined. Instead of 0, 0, 0, 1, 1, 1 it has to be 0, 0, 0, 1, 2.25, 1.

Thanks!

Another point to consider is that we might want to retrieve only the fracture network. For these cases, creating the mdg will be quite time-consuming. Would you consider adding an only_network: bool = False input parameter to cover such cases?

I don't see the use case that would justify the . The data file is available and can be loaded with 1-2 lines of code. Am I missing something.

Finally, this might be a rare case, but: How do we know that there is a 1-to-1 match between the fracture network and the mdg? > Both, in terms of coordinates and indexing.

With a grid imported from a pre-generated .geo file, we cannot.

[keileg]

In my understanding, pytest fails for the following reason: Before running GH actions, PorePy is installed as a standard package (not in editable mode as we usually do) and placed in a directory .../python3.11/site-packages/porepy/. The .geo file is not part of the pip installation, thus it is only available via the repository. In mdg_library.benchmark_3d_case_3(), the data file is specified as a relative path, which apparently is interpreted based on the location of the installed files (site-packages etc.). This does not work.

Note that this, AFAIK, is the only test that uses pre-generated data files, thus this is a problem we have not encountered previously. Hence, the solution we end up with may set precedence for other cases, though I doubt there will be many of them.

Options:

1. Make GH actions install in editable mode. This would cure the symptom, but would require any user who wants to generate this mesh to also install in editable mode, which is hardly ideal.
2. Somehow make the file available in the test, perhaps by copying files around on the GH server. This will have the same problem for users.
3. Remove this test. Same problem.
4. Check if the file is present, and if not, download it from GH and place it somewhere accessible. This is hardly beautiful, but it will at least work.
5. Decide that, after all, we do not want to support this geometry. The same will by extension apply to other geometries that cannot readily be specified by the standard PorePy approach, and for any other future functionality that may rely on access to data files (as part of the repository).

I don't like any of these options, but no 4 is the only one that will not crash or set strict constraints on future development.

@jhabriel @IvarStefansson @OmarDuran:

1. Does my reasoning make sense, or do you see other explanation for this error messsage?
2. Do you have other suggestions

[jhabriel]

I'm not so sure this has to do with porepy being installed in editable mode or not. I believe the problem is that Actions cannot find the geo files since it interprets that it has to be provided by the porepy installation. Since Actions installs the last develop version, it simply does not notice the newly added geo files.

A potential fix here could be to make the geo files available in a separate PR so that the dev branch is updated. Again, this might or might not work.

P.D.: If you check the Install PorePy step in the failing check, you will see that all packages are stored in the site_packages directory.

[IvarStefansson]

In my understanding, pytest fails for the following reason: Before running GH actions, PorePy is installed as a standard package (not in editable mode as we usually do) and placed in a directory .../python3.11/site-packages/porepy/. The .geo file is not part of the pip installation, thus it is only available via the repository. In mdg_library.benchmark_3d_case_3(), the data file is specified as a relative path, which apparently is interpreted based on the location of the installed files (site-packages etc.). This does not work.

Note that this, AFAIK, is the only test that uses pre-generated data files, thus this is a problem we have not encountered previously. Hence, the solution we end up with may set precedence for other cases, though I doubt there will be many of them.

Options:

1. Make GH actions install in editable mode. This would cure the symptom, but would require any user who wants to generate this mesh to also install in editable mode, which is hardly ideal.

2. Somehow make the file available in the test, perhaps by copying files around on the GH server. This will have the same problem for users.

3. Remove this test. Same problem.

4. Check if the file is present, and if not, download it from GH and place it somewhere accessible. This is hardly beautiful, but it will at least work.

5. Decide that, after all, we do not want to support this geometry. The same will by extension apply to other geometries that cannot readily be specified by the standard PorePy approach, and for any other future functionality that may rely on access to data files (as part of the repository).

I don't like any of these options, but no 4 is the only one that will not crash or set strict constraints on future development.

@jhabriel @IvarStefansson @OmarDuran:

1. Does my reasoning make sense, or do you see other explanation for this  [error messsage](https://github.com/pmgbergen/porepy/actions/runs/7538888665/job/20520278738?pr=1096)?

2. Do you have other suggestions

As you know, this is not exactly my home turf. But could it be that this can be fixed in setup.py? See this: https://stackoverflow.com/questions/11848030/how-include-static-files-to-setuptools-python-package

[keileg]

I'm not so sure this has to do with porepy being installed in editable mode or not. I believe the problem is that Actions cannot >find the geo files since it interprets that it has to be provided by the porepy installation. Since Actions installs the last develop version, it simply does not notice the newly added geo files.

If you look at the path where python expects the data file to be, it is at a location which
i) is set relative to the location of the file trying to load the data file
ii) is outside the repository.

Since the source file has been moved, presumably due to the installation process, but the data file has not, this is bound to fail. We should not try to give the correct relative path, that would be bad practice for so many reasons. The .geo files are already available (or else the updated test would not have been available), so splitting this into two PRs will not make the problem go away.

[jhabriel]

Oh, I understand the problem now. In that case, I would go for option 4, unless there exists a more elegant alternative.

[IvarStefansson]

It seems my suggestion might work. See this test run on this branch, which is based off of the branch of this PR. Note that I tried and failed at doing something a bit more targeted (specific folders only). My attempts are documented in the branch's git history. I am sure we can find a way to make that work given a bit more stubbornness and technical expertise.

[keileg]

It seems my suggestion might work. See this test run on this branch, which is based off of the branch of this PR. Note that I tried and failed at doing something a bit more targeted (specific folders only). My attempts are documented in the branch's git history. I am sure we can find a way to make that work given a bit more stubbornness and technical expertise.

Thanks, @IvarStefansson, this looks promising. I will go ahead with your suggestion, but as you suggest, also read up a bit more before finalizing.

[keileg]

@IvarStefansson I took your suggestion, which seems like an accepted solution on the internet, but narrowed the scope down to only files living within the directory of gmsh_files. We can of course expand later, but for now, caution seems preferrable.

[keileg]

With the latest changes, everything should be fine now.