-
Notifications
You must be signed in to change notification settings - Fork 239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow multiple meshes in project #2156
Allow multiple meshes in project #2156
Conversation
I'm a little stuck with the parallelization part here. As soon as this is available, we can continue on the review... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far it looks fine. Only minor comments.
auto optional_meshes = config.getConfigSubtreeOptional("meshes"); | ||
if (optional_meshes) | ||
{ | ||
DBUG("OK Reading multiple meshes."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe drop OK
.
if (optional_meshes) | ||
{ | ||
DBUG("OK Reading multiple meshes."); | ||
for (auto mesh_config_line : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe only mesh_config
.
{ // Read single mesh with geometry. | ||
WARN( | ||
"Consider switching from mesh and geometry input to multiple " | ||
"meshes input."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, pointing to some docu would be nice. TBD later? Add TODO?
auto pv = | ||
ProcessLib::ProcessVariable{var_config, _mesh_vec, _parameters}; | ||
// Either the mesh name is given, or the first mesh's name will be | ||
// taken. Taking the first mesh's value is depricated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deprecated
ProcessLib::ProcessVariable{var_config, _mesh_vec, _parameters}; | ||
// Either the mesh name is given, or the first mesh's name will be | ||
// taken. Taking the first mesh's value is depricated. | ||
std::string const mesh_name = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were times when you used to use auto
much more often. ^^
ProcessLib/ProcessVariable.cpp
Outdated
@@ -30,6 +30,12 @@ MeshLib::Mesh const& findMeshInConfig( | |||
// | |||
std::string mesh_name; // Either given directly in <mesh> or constructed | |||
// from <geometrical_set>_<geometry>. | |||
|
|||
#ifdef DOCUMENTATION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe rename the macro, e.g., OGS_THIS_IS_DOXYGEN_RUN
.
example in LIE, where the displacement jump variable is defined only on the | ||
fractures, or in case of hydro mechanics, where the pressure variable is defined | ||
on lower order elements, the boundary mesh is not strictly required to coincide | ||
with the part of the process variable's boundary part. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it's not checked if they coincide, but only if they have the same number of nodes, is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is assumed that geometrically the boundary mesh is fitting the bulk mesh. This is not checked, but "guaranteed" if the constructMeshesFromGeometries tool is used.
Now, if the strict_bc_domain is set, then all nodes of the bc_mesh for that variable must have a valid entry in the DOF table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I wanted to say. 😄
And I'd add the following to the documentation:
It is assumed that geometrically the boundary mesh is fitting the bulk mesh.
@@ -75,9 +75,17 @@ createNonuniformNeumannBoundaryCondition( | |||
mapping_to_bulk_nodes_property.c_str()); | |||
} | |||
|
|||
auto const strict_bc_domain = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't that be moved to a central place for all BCs?
Documentation/Doxyfile.in
Outdated
@@ -2023,7 +2023,7 @@ INCLUDE_FILE_PATTERNS = | |||
# recursively expanded use the := operator instead of the = operator. | |||
# This tag requires that the tag ENABLE_PREPROCESSING is set to YES. | |||
|
|||
PREDEFINED = | |||
PREDEFINED = DOCUMENTATION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe rename the macro
@@ -86,6 +86,21 @@ | |||
</xs:complexType> | |||
</xs:element> | |||
<xs:element name="geometry" type="xs:string" minOccurs="0"/> | |||
<xs:element name="meshes" minOccurs="0"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maxOccurs="1"
or is that the default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Only small stuffs.
@@ -0,0 +1 @@ | |||
\ogs_missing_documentation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing documentation.
NumLib/DOF/LocalToGlobalIndexMap.h
Outdated
/// The \c strict_bc_domain flag is controlling if the geometrical boundary | ||
/// domain and the domain of the global components (varaible_id, | ||
/// component_ids) must coincide. If set to false the not found global | ||
/// indices will not inserted into the derived DOF table. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will not be
config.getConfigParameter<bool>("strict_bc_domain", true); | ||
if (!strict_bc_domain) | ||
{ | ||
DBUG("Allow non-strict bc domain and global component mapping."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bc --> BC?
@@ -45,6 +39,23 @@ GenericNaturalBoundaryCondition<BoundaryConditionData, | |||
dof_table_bulk.getNumberOfVariableComponents(variable_id)); | |||
} | |||
|
|||
if (_bc_mesh.getDimension() + 1 != global_dim) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about the case of a 3D mesh getting 1D BC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With out "ordinary" natural BCs a 1D BC for a 3D domain doesn't make sense, does it? For something like line sources the case is different, of course. But they don't fit our natural BCs anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that BCs on lines for 3D problem are excluded from this type of definition.
} | ||
|
||
if (_boundary_mesh->getDimension() + 1 != global_dim) | ||
if (_bc_mesh.getDimension() + 1 != global_dim) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same question.
config.getConfigParameter<bool>("strict_bc_domain", true); | ||
if (!strict_bc_domain) | ||
{ | ||
DBUG("Allow non-strict bc domain and global component mapping."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bc-->BC?
|
||
if (!boundary_mesh) | ||
// Surface mesh and bulk mesh must have equal axial symmetry flags! | ||
if (boundary_mesh.isAxiallySymmetric() != bulk_mesh.isAxiallySymmetric()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw this twice. Is it possible to set it as a function?
n_and_weight.N, flux); | ||
_local_rhs.noalias() += | ||
n_and_weight.N * (n_and_weight.weight * flux); | ||
// pos.setIntegrationPoint(ip); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the comment needed?
Removing most of the code duplication in the BCs/STs creation.
Rename bulk_node_ids and bulk_element_ids. Use multiple meshes for setting the (heterogeneous) BCs.
This is needed to repeat parts of the code solely for the documentation purposes.
Used in construction of mesh names in two places. This unifies the name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
OpenGeoSys development has been moved to GitLab. |
follow up of ctest fixes #2155
Main features are:
possibility (instead of a single mesh and geometry) to the project file.