Open pschultzendorff opened 1 month ago
Thanks for volunteering for this, @pschultzendorff. Short comments:
cfl
has been used for years, so if you decide it is easier to delete it than to fix it, I for one will not object.
- I am not sure if the upwind class will stay relevant when the compositional flow branch is introduced. Could you comment briefly, @vlipovac?
It is used in the advective part of energy and transport equations. Considering also our recent discussion on having the possibility to run a simulation without re-discretizing MPFA, it will remain relevant.
On a sidenote, there are open inconsistencies or unclarities in that class regarding BC (it needs an own instance of BoundaryCondition
and , to my understanding, that instance needs all boundary faces to be flagged as 'dir'
)
Good to hear that it will stay relevant, as I definitely need it for my two-phase flow code :D
I assume you are referring to #760 regarding the inconsistencies @vlipovac? For my part, I also ran into some inconsistencies/unwanted behavior of the BC when using it for modeling flow and transport in the fractional flow formulation. However, there are some more or less hacky ways to get it to work the way I need, so this should not be part of the issue.
I will fix the documentation as best as possible.
The method
pp.numerics.fv.upwind.Upwind.discretize
has quite outdated documentation. For example, the required keys in the parameter dictionary as well as the return values of the method are wrong. In addition, #1192 changed the signs of the boundary matrices, but the code comments reflect the old convention. The other methods seem better, but, e.g.,Upwind.cfl
(not sure if this is even actively used) indicates also wrong dictionary keys.