search

pmgbergen / porepy

Python Simulation Tool for Fractured and Deformable Porous Media
GNU General Public License v3.0
251 stars 88 forks source link

Entry barrier for setting up simple models like heat equation is too high #1174

Closed vlipovac closed 5 months ago

vlipovac commented 5 months ago

This issue came up during an advisory meeting with a trainee at the CSD, and consists of two parts.

  1. The module porepy.models.energy_balance.py is designed to be an addition to flow and transport models, not to be a stand-alone module for solving the heat equation. Some mixins and inheritance relations are missing (f.e. VariablesEnergyBalance does not inherit from VariablesMixin, while VariablesSinglePhaseFlow does)

  2. The model set-up requires too many unnecessary mixins, especially for the fixed-dimensional case. In the case of the heat equation, I identified two sections in the code causing this: a. The base models always call the methods assembling interface and well equations with an empty sequence of lower-dimensional grids. I observed this in all base models. This might also lead to confusion when inspecting the equation system, where empty equations are added. b. The solution strategy of the energy balance does not adapt to the case when there is no advective flux, calling methods like darcy_flux which need to be provided returning e.g. a wrapped zero array.

I am not sure though, if point 2.a is something which should be fixed, or if this is something the user has to keep in mind when setting up models.

IvarStefansson commented 5 months ago

For now, like you say, the energy balance model is simply not designed for standalone use. I don't think there are imminent plans to redesign it. Do you think the documentation needs clarification on this point?

vlipovac commented 5 months ago

For now, like you say, the energy balance model is simply not designed for standalone use. I don't think there are imminent plans to redesign it. Do you think the documentation needs clarification on this point?

The documentation in the module docstring points that out, and I think it is fine.

What are your thoughts on the second point. that the models always provide all equations, even in the fixed-dimensional case? Of course the total flops are not increased severely, since the arrays are empty, but they appear in the Operator tree (recursion during _evaluate) and in the equation system, despite being empty.

keileg commented 5 months ago

This is a fair question, which should be addressed as part of a larger maintenance of the equation and operator framework.

keileg commented 5 months ago

I wrote down some thoughts in this direction in #1184. @vlipovac is it okay with you if we close this issue and continue the discussion there?