dumux-repositories issueshttps://git.iws.uni-stuttgart.de/groups/dumux-repositories/-/issues2019-09-13T15:06:18Zhttps://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/764[multidomain][boundary][stokesdarcy] couplingmapper does not need couplingman...2019-09-13T15:06:18ZKatharina Heck[multidomain][boundary][stokesdarcy] couplingmapper does not need couplingmanager<!--
This form is for feature requests ONLY!
If you're looking for help check out the [readme](/README.md).
-->
**Feature request**
The couplingmapper could be simplified and not depend on the couplingmanager. The couplingmanager is only needed to get the FVGridgeometries in one function, which seems unnecessary.
**What does this feature / why does DuMux need it**:
The dependency of the mapper on the couplingmanager create issues when trying to combine several couplingmangers. Removing that also means that the constructor of the couplingmanager can be removed, which is also nice and easier for inheritance.<!--
This form is for feature requests ONLY!
If you're looking for help check out the [readme](/README.md).
-->
**Feature request**
The couplingmapper could be simplified and not depend on the couplingmanager. The couplingmanager is only needed to get the FVGridgeometries in one function, which seems unnecessary.
**What does this feature / why does DuMux need it**:
The dependency of the mapper on the couplingmanager create issues when trying to combine several couplingmangers. Removing that also means that the constructor of the couplingmanager can be removed, which is also nice and easier for inheritance.3.2https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/763[FreeFlow] No Newton convergence for pressure Dirichlet BCs at high Re2019-09-05T07:39:06ZKilian Weishauptkilian.weishaupt@iws.uni-stuttgart.de[FreeFlow] No Newton convergence for pressure Dirichlet BCs at high ReFor simple pipe flow, setting Dirichlet BCs for the pressure both at the inlet and outlet
works well for low Re. For higher Re, the Newton scheme does not converge anymore.
Setting the inlet BCs to Dirichlet for velocity works.
This should be investigated. Maybe the assumption of
$`\nabla \mathbf{v} \cdot \mathbf{n} = 0 `$
and
$`\nabla p \cdot \mathbf{n} = 0 `$
used for the pressure BC causes this problem?For simple pipe flow, setting Dirichlet BCs for the pressure both at the inlet and outlet
works well for low Re. For higher Re, the Newton scheme does not converge anymore.
Setting the inlet BCs to Dirichlet for velocity works.
This should be investigated. Maybe the assumption of
$`\nabla \mathbf{v} \cdot \mathbf{n} = 0 `$
and
$`\nabla p \cdot \mathbf{n} = 0 `$
used for the pressure BC causes this problem?3.2https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/762Dumux release 3.12019-09-04T06:38:08ZKatharina HeckDumux release 3.1All major changes that should be included and are not yet committed should be announced.
Add a comment here describing the feature and areas affected by the change.All major changes that should be included and are not yet committed should be announced.
Add a comment here describing the feature and areas affected by the change.3.12019-10-11Katharina HeckKatharina Heckhttps://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/761Cleanup explicit flash of implicit 2p2c model2019-08-30T08:46:24ZBeatrix BeckerCleanup explicit flash of implicit 2p2c modelThe volumevariables of the 2p2c model have an explicit flash directly implemented in the volumevariables itself.
* In general I like the idea of a specialized 2p2c flash that is easy to understand and fast, but it shouldn't be included in the volumevariables. I would propose implementing it as a separate class in a separate header, like the other constraintsolvers.
* The flash is only used if `useConstraintSolver` is false and the default is true. For the 2p2c model I would make the flash the default, since it is a faster calculation than using the more general `MiscibleMultiPhaseComposition` constraintsolver which solves a linear system of equations. Maybe we should even completely delete `useConstraintSolver` because in my opinion the solver has no benefit here, it solves the same equations, just less efficiently.
* For the case of one phase we may use the `ComputeFromReferencePhase` constraintsolver since it does exactly what the flash does.
* I don't think the flash is currently tested, so this should be added. It should have the same result as the other constraintsolvers.
What do you think? Another solution could be to delete the flash code and always use the solvers that we already have, but as mentioned above, I prefer having a 2p2c-specific flash.
There are a few points that I'm not sure of, maybe @holle can comment on this:
* In my opinion this flash is not as correct as it could be because it uses the assumption that vapor pressure of the liquid component and partial pressure of the liquid component in the gas phase are the same. This is only the case if we neglect the presence of other components in the gas phase. There is an equally quick method to calculate the mass fractions without using this assumption, see the 2p2c flash of the sequential models.
* It seems that the flash assumes that we deal with one liquid and one gas phase and that the liquid phase is the first phase. I think the 2p2c flash of the sequential models doesn't have this constraint.
* There seems to be a bug in the case that only the first phase is present, in the calculation of the mole fraction of the first component in the second phase. A multiplication with the mole fraction of the first component in the first phase is probably missing.The volumevariables of the 2p2c model have an explicit flash directly implemented in the volumevariables itself.
* In general I like the idea of a specialized 2p2c flash that is easy to understand and fast, but it shouldn't be included in the volumevariables. I would propose implementing it as a separate class in a separate header, like the other constraintsolvers.
* The flash is only used if `useConstraintSolver` is false and the default is true. For the 2p2c model I would make the flash the default, since it is a faster calculation than using the more general `MiscibleMultiPhaseComposition` constraintsolver which solves a linear system of equations. Maybe we should even completely delete `useConstraintSolver` because in my opinion the solver has no benefit here, it solves the same equations, just less efficiently.
* For the case of one phase we may use the `ComputeFromReferencePhase` constraintsolver since it does exactly what the flash does.
* I don't think the flash is currently tested, so this should be added. It should have the same result as the other constraintsolvers.
What do you think? Another solution could be to delete the flash code and always use the solvers that we already have, but as mentioned above, I prefer having a 2p2c-specific flash.
There are a few points that I'm not sure of, maybe @holle can comment on this:
* In my opinion this flash is not as correct as it could be because it uses the assumption that vapor pressure of the liquid component and partial pressure of the liquid component in the gas phase are the same. This is only the case if we neglect the presence of other components in the gas phase. There is an equally quick method to calculate the mass fractions without using this assumption, see the 2p2c flash of the sequential models.
* It seems that the flash assumes that we deal with one liquid and one gas phase and that the liquid phase is the first phase. I think the 2p2c flash of the sequential models doesn't have this constraint.
* There seems to be a bug in the case that only the first phase is present, in the calculation of the mole fraction of the first component in the second phase. A multiplication with the mole fraction of the first component in the first phase is probably missing.3.1Beatrix BeckerBeatrix Beckerhttps://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/760Tests for flashs don't test anything2019-08-29T12:59:01ZBeatrix BeckerTests for flashs don't test anythingThey never fail, just print an error.They never fail, just print an error.3.1Beatrix BeckerBeatrix Beckerhttps://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/759compositionalflash.hh: iterate saturation and pressure within flash2019-08-28T15:13:56ZBeatrix Beckercompositionalflash.hh: iterate saturation and pressure within flashThe function `concentrationFlash2p2c` gets both phase pressures as input. Since the saturation is unknown, one of these pressures (more precisely, the capillary pressure) is unknown, too. In the code the pressure is iterated outside of the flash, e.g., in the routine that updates the secondary variables, and the flash is called several times during that iteration. This is not consistent with the implicit code, in my opinion, and also not very convenient. The flash should take care of iterating saturation and pressure, in my opinion.The function `concentrationFlash2p2c` gets both phase pressures as input. Since the saturation is unknown, one of these pressures (more precisely, the capillary pressure) is unknown, too. In the code the pressure is iterated outside of the flash, e.g., in the routine that updates the secondary variables, and the flash is called several times during that iteration. This is not consistent with the implicit code, in my opinion, and also not very convenient. The flash should take care of iterating saturation and pressure, in my opinion.3.2https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/758compositionalflash.hh: porosity is not used2019-08-28T15:03:37ZBeatrix Beckercompositionalflash.hh: porosity is not usedSome functions in CompositionalFlash require porosity as input, which is never used. Needs to be deleted and deprecated.Some functions in CompositionalFlash require porosity as input, which is never used. Needs to be deleted and deprecated.3.2https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/757Reformulate diffusion laws to match with mass reference velocities2019-09-05T09:54:14ZKatharina HeckReformulate diffusion laws to match with mass reference velocitiesAs discussed previously: if Darcys law and Navier Stokes law give mass averaged velocities we need to adapt the diffusion laws to calculate the gradient based on mass fractions not mole fractions.As discussed previously: if Darcys law and Navier Stokes law give mass averaged velocities we need to adapt the diffusion laws to calculate the gradient based on mass fractions not mole fractions.3.1Katharina HeckKatharina Heckhttps://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/756Periodic Boundary Conditions for FreeFlow2019-08-29T04:38:01ZLars KaiserPeriodic Boundary Conditions for FreeFlow<!--
This form is for feature requests ONLY!
If you're looking for help check out the [readme](/README.md).
-->
**Feature request**
Periodic Boundary Conditions for FreeFlow
**What does this feature / why does DuMux need it**:
Periodic Boundary Conditions are a common and useful BC and help to overcome the problem of finding appropriate BC for finite domains.
**Which issue does this feature fix (if any)**
It would enable using Periodic Boundary Conditions in a coupled Darcy-FreeFlow model.
Periodic Boundary Conditions are already implemented for Darcy's law but can't be used reasonable in a coupled model with freeflow, because freeflow doesn't allow Periodic Boundary Conditions.
**Anything else we need to know?**:
Already mentioned in dumux-repositories/dumux#487<!--
This form is for feature requests ONLY!
If you're looking for help check out the [readme](/README.md).
-->
**Feature request**
Periodic Boundary Conditions for FreeFlow
**What does this feature / why does DuMux need it**:
Periodic Boundary Conditions are a common and useful BC and help to overcome the problem of finding appropriate BC for finite domains.
**Which issue does this feature fix (if any)**
It would enable using Periodic Boundary Conditions in a coupled Darcy-FreeFlow model.
Periodic Boundary Conditions are already implemented for Darcy's law but can't be used reasonable in a coupled model with freeflow, because freeflow doesn't allow Periodic Boundary Conditions.
**Anything else we need to know?**:
Already mentioned in dumux-repositories/dumux#4873.1https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/755[tabularization] Tabularization doesn't use the same spacing everywhere when ...2019-08-28T09:51:54ZTimo Kochtimo.koch@iws.uni-stuttgart.de[tabularization] Tabularization doesn't use the same spacing everywhere when useVaporPressure=trueFor use vaporPressure the liquid and gaseous states are tabularized separately both using the specified number of sample points. That leads to very different spacing throughout the parameter space. It would be more consistent to compute a spacing from the entire specified range and the specified number of sampling points and then use the same spacing to tabularize sub-spaces. That should also lead (with some tolerance) the total number of sampling point that the user specified.For use vaporPressure the liquid and gaseous states are tabularized separately both using the specified number of sample points. That leads to very different spacing throughout the parameter space. It would be more consistent to compute a spacing from the entire specified range and the specified number of sampling points and then use the same spacing to tabularize sub-spaces. That should also lead (with some tolerance) the total number of sampling point that the user specified.3.2https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/754Automatic Differentiation2019-08-28T08:24:16ZNed ColtmanAutomatic Differentiation<!--
This form is for feature requests ONLY!
If you're looking for help check out the [readme](/README.md).
-->
**Feature request**
Automatic Differentiation (AD)
**What does this feature / why does DuMux need it**:
Often used in other software packages, should be more accurate than numerical differentiation
**Which issue does this feature fix (if any)**
Removes numerical differentiation imprecision, more similar to a hand built jacobian.
**Anything else we need to know?**:
OPM has implemented this already, and their method seems compatible with Dumux. Dumux AD could likely be copied from OPM for the most part.
OPM does Forward AD, set up with operator overloading.
See MR (...)<!--
This form is for feature requests ONLY!
If you're looking for help check out the [readme](/README.md).
-->
**Feature request**
Automatic Differentiation (AD)
**What does this feature / why does DuMux need it**:
Often used in other software packages, should be more accurate than numerical differentiation
**Which issue does this feature fix (if any)**
Removes numerical differentiation imprecision, more similar to a hand built jacobian.
**Anything else we need to know?**:
OPM has implemented this already, and their method seems compatible with Dumux. Dumux AD could likely be copied from OPM for the most part.
OPM does Forward AD, set up with operator overloading.
See MR (...)3.2Ned ColtmanNed Coltmanhttps://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/753[tests] Newon line search not tested in test suite2019-08-28T09:54:42ZTimo Kochtimo.koch@iws.uni-stuttgart.de[tests] Newon line search not tested in test suiteWe could just enable if for some test, ideally where it also improves convergence.We could just enable if for some test, ideally where it also improves convergence.3.2https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/752[test] Mpfa not tested in parallel2019-08-28T09:54:51ZTimo Kochtimo.koch@iws.uni-stuttgart.de[test] Mpfa not tested in parallelWe could add the richards parallel test for mpfa too. Solution should be identical to tpfa.We could add the richards parallel test for mpfa too. Solution should be identical to tpfa.3.2https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/751[test] Unit test intersectspointgeometry incomplete2019-08-28T09:54:58ZTimo Kochtimo.koch@iws.uni-stuttgart.de[test] Unit test intersectspointgeometry incompletepoint-tetrahedron and point-pyramid is not tested
Maybe a dedicated unit test for this header would be good and easy to implement.point-tetrahedron and point-pyramid is not tested
Maybe a dedicated unit test for this header would be good and easy to implement.3.2https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/749Default usage message not up-to-date2019-08-27T16:53:31ZTimo Kochtimo.koch@iws.uni-stuttgart.deDefault usage message not up-to-dateIt still contains TimeManager and PrintProperties...
https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/blob/master/dumux/common/defaultusagemessage.hh
It's also never tested that it actually gets triggered. Could be done in a tiny unit test.It still contains TimeManager and PrintProperties...
https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/blob/master/dumux/common/defaultusagemessage.hh
It's also never tested that it actually gets triggered. Could be done in a tiny unit test.3.1https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/745Implement HasResize2019-08-28T09:56:23ZTimo Kochtimo.koch@iws.uni-stuttgart.deImplement HasResizeThe following discussion from !1546 should be addressed:
- [ ] @timok started a [discussion](https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/merge_requests/1546#note_28434):
> I think has resize is also needed in other places. Theoretically we can just use a copy of https://web.math.pmf.unizg.hr/nastava/nrpdj/DuneDoc/release-2.6.1/dune-functions/doc/doxygen/html/a01948_source.html
> the implementation in dune functions and put it e.g. in dumux/common/concepts.hh
Also some other tiny "concepts" might be useful, however usually it's not real concepts so we should be careful.The following discussion from !1546 should be addressed:
- [ ] @timok started a [discussion](https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/merge_requests/1546#note_28434):
> I think has resize is also needed in other places. Theoretically we can just use a copy of https://web.math.pmf.unizg.hr/nastava/nrpdj/DuneDoc/release-2.6.1/dune-functions/doc/doxygen/html/a01948_source.html
> the implementation in dune functions and put it e.g. in dumux/common/concepts.hh
Also some other tiny "concepts" might be useful, however usually it's not real concepts so we should be careful.3.2https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/743Strongly enforced Dirichlet constraints for cell-centered (internal, core+mul...2019-08-28T08:36:06ZTimo Kochtimo.koch@iws.uni-stuttgart.deStrongly enforced Dirichlet constraints for cell-centered (internal, core+multidomain) not correct<!--
This form is for bug reports ONLY!
If you're looking for help check out the [readme](/README.md).
-->
**Bug report**
* ~~For box multidomain:~~
~~If Dirichlet is set for a dof that also has a coupling derivative the entire matrix line has to be set to zero. Currently this is only done for the diagonal block. This bug might be actually in use for the boundary coupling code where this kind of incorporation is wanted, but also this is not a Dirichlet constraint (equation is not replaced by the Dirichlet constraint but by the coupling condition).~~ Edit: I was mistakes and this is already implemented.
* For cell-centered (internal Dirichlet):
The Dirichlet conditions are currently enforced in the local assembler. However due to the assembly procedure the matrix rows are overwritten again later in the assembly of other elements. In order to fix this the constraints need to be either implemented after the whole matrix has been assembled (local caches cannot be reused), or (more complicated to read but maybe more efficient) they have to be incorporated during the assembly (don't write anything in rows of Dirichlet dofs)
**What happened / Problem description**:
Jacobian is corrupted.
**What you expected to happen**:
Dirichlet constraints are strongly enforced in the matrix.
**How to reproduce it (as minimally and precisely as possible)**:
The internal Dirichlet test for cell-centered.
**Anything else we need to know?**:
We should think about a better solution how and when to incorporate Dirichlet constraints.
**Environment**:
- DuMux version: master (the cell-centered part was only instroduced recently !1664)<!--
This form is for bug reports ONLY!
If you're looking for help check out the [readme](/README.md).
-->
**Bug report**
* ~~For box multidomain:~~
~~If Dirichlet is set for a dof that also has a coupling derivative the entire matrix line has to be set to zero. Currently this is only done for the diagonal block. This bug might be actually in use for the boundary coupling code where this kind of incorporation is wanted, but also this is not a Dirichlet constraint (equation is not replaced by the Dirichlet constraint but by the coupling condition).~~ Edit: I was mistakes and this is already implemented.
* For cell-centered (internal Dirichlet):
The Dirichlet conditions are currently enforced in the local assembler. However due to the assembly procedure the matrix rows are overwritten again later in the assembly of other elements. In order to fix this the constraints need to be either implemented after the whole matrix has been assembled (local caches cannot be reused), or (more complicated to read but maybe more efficient) they have to be incorporated during the assembly (don't write anything in rows of Dirichlet dofs)
**What happened / Problem description**:
Jacobian is corrupted.
**What you expected to happen**:
Dirichlet constraints are strongly enforced in the matrix.
**How to reproduce it (as minimally and precisely as possible)**:
The internal Dirichlet test for cell-centered.
**Anything else we need to know?**:
We should think about a better solution how and when to incorporate Dirichlet constraints.
**Environment**:
- DuMux version: master (the cell-centered part was only instroduced recently !1664)3.1Timo Kochtimo.koch@iws.uni-stuttgart.deTimo Kochtimo.koch@iws.uni-stuttgart.dehttps://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/740Enforce constraints in fluid state or algorithm?2019-08-28T08:40:50ZBeatrix BeckerEnforce constraints in fluid state or algorithm?Currently only one mass fraction has to be set for the compositional fluid state, the other one is internally deduced from that. This only works for two component models. In general: is it a good idea that the fluid state enforces such constraints or wouldn't it be better if the algorithm took care of that? @timok suggests that the fluid state could have a function like checkSumMoleFractions(Scalar value=1.0) to be able to assert the constraint before an algorithm that has that assumption about a fluid state.Currently only one mass fraction has to be set for the compositional fluid state, the other one is internally deduced from that. This only works for two component models. In general: is it a good idea that the fluid state enforces such constraints or wouldn't it be better if the algorithm took care of that? @timok suggests that the fluid state could have a function like checkSumMoleFractions(Scalar value=1.0) to be able to assert the constraint before an algorithm that has that assumption about a fluid state.3.2https://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/739Unit test for compositional flash2019-07-15T15:14:19ZBeatrix BeckerUnit test for compositional flashThere is none currentlyThere is none currently3.1Beatrix BeckerBeatrix Beckerhttps://git.iws.uni-stuttgart.de/dumux-repositories/dumux/issues/737Creating a bounding box tree for a vector of geometries2019-08-28T09:56:45ZTimo Kochtimo.koch@iws.uni-stuttgart.deCreating a bounding box tree for a vector of geometries<!--
This form is for feature requests ONLY!
If you're looking for help check out the [readme](/README.md).
-->
**Feature request**
Bounding box tree for a list of geometries without a grid
**What does this feature / why does DuMux need it**:
E.g. to intersect a polyline with a grid or some object where it would be
easier to just define the geometries instead of creating a Dune grid.
The bounding box tree is already general enough, it expects a EntitySet which only needs
to satisfy a certain subset of the interface of a Dune::GridView. So the task would be
to create a new EntitySet class which can be constructed from a vector of geometries instead
of from a grid view.<!--
This form is for feature requests ONLY!
If you're looking for help check out the [readme](/README.md).
-->
**Feature request**
Bounding box tree for a list of geometries without a grid
**What does this feature / why does DuMux need it**:
E.g. to intersect a polyline with a grid or some object where it would be
easier to just define the geometries instead of creating a Dune grid.
The bounding box tree is already general enough, it expects a EntitySet which only needs
to satisfy a certain subset of the interface of a Dune::GridView. So the task would be
to create a new EntitySet class which can be constructed from a vector of geometries instead
of from a grid view.3.2