Skip to content
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

Merged
merged 30 commits into from Aug 23, 2018

Conversation

Projects
None yet
3 participants
@endJunction
Copy link
Member

endJunction commented Jul 1, 2018

follow up of ctest fixes #2155

Main features are:

  1. the addition of
   <meshes>
      <mesh>a.vtu</mesh>
      ...
   </meshes>

possibility (instead of a single mesh and geometry) to the project file.

  1. Using the mesh names in non-uniform boundary conditions.

@endJunction endJunction force-pushed the endJunction:AllowMultipleMeshesInProject_j branch 4 times, most recently from 8a0e871 to a5cf05d Jul 2, 2018

@endJunction endJunction added the WIP 🏗 label Jul 5, 2018

@endJunction

This comment has been minimized.

Copy link
Member Author

endJunction commented Jul 5, 2018

I'm a little stuck with the parallelization part here. As soon as this is available, we can continue on the review...

@endJunction endJunction force-pushed the endJunction:AllowMultipleMeshesInProject_j branch 4 times, most recently from 3bdf44b to c5c2bc1 Jul 6, 2018

@chleh
Copy link
Collaborator

chleh left a comment

So far it looks fine. Only minor comments.

auto optional_meshes = config.getConfigSubtreeOptional("meshes");
if (optional_meshes)
{
DBUG("OK Reading multiple meshes.");

This comment has been minimized.

@chleh

chleh Jul 9, 2018

Collaborator

Maybe drop OK.

if (optional_meshes)
{
DBUG("OK Reading multiple meshes.");
for (auto mesh_config_line :

This comment has been minimized.

@chleh

chleh Jul 9, 2018

Collaborator

Maybe only mesh_config.

{ // Read single mesh with geometry.
WARN(
"Consider switching from mesh and geometry input to multiple "
"meshes input.");

This comment has been minimized.

@chleh

chleh Jul 9, 2018

Collaborator

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.

This comment has been minimized.

@chleh

chleh Jul 9, 2018

Collaborator

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 =

This comment has been minimized.

@chleh

chleh Jul 9, 2018

Collaborator

There were times when you used to use auto much more often. ^^

@@ -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

This comment has been minimized.

@chleh

chleh Jul 9, 2018

Collaborator

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.

This comment has been minimized.

@chleh

chleh Jul 9, 2018

Collaborator

Actually it's not checked if they coincide, but only if they have the same number of nodes, is it?

This comment has been minimized.

@endJunction

endJunction Jul 13, 2018

Author Member

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.

This comment has been minimized.

@chleh

chleh Jul 17, 2018

Collaborator

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 =

This comment has been minimized.

@chleh

chleh Jul 9, 2018

Collaborator

can't that be moved to a central place for all BCs?

@@ -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

This comment has been minimized.

@chleh

chleh Jul 9, 2018

Collaborator

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">

This comment has been minimized.

@chleh

chleh Jul 9, 2018

Collaborator

maxOccurs="1" or is that the default?

@endJunction endJunction referenced this pull request Jul 13, 2018

Merged

Algorithms #2161

@wenqing
Copy link
Member

wenqing left a comment

Looks good. Only small stuffs.

@@ -0,0 +1 @@
\ogs_missing_documentation

This comment has been minimized.

@wenqing

wenqing Jul 13, 2018

Member

missing documentation.

/// 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.

This comment has been minimized.

@wenqing

wenqing Jul 13, 2018

Member

will not be

config.getConfigParameter<bool>("strict_bc_domain", true);
if (!strict_bc_domain)
{
DBUG("Allow non-strict bc domain and global component mapping.");

This comment has been minimized.

@wenqing

wenqing Jul 13, 2018

Member

bc --> BC?

@@ -45,6 +39,23 @@ GenericNaturalBoundaryCondition<BoundaryConditionData,
dof_table_bulk.getNumberOfVariableComponents(variable_id));
}

if (_bc_mesh.getDimension() + 1 != global_dim)

This comment has been minimized.

@wenqing

wenqing Jul 13, 2018

Member

How about the case of a 3D mesh getting 1D BC?

This comment has been minimized.

@chleh

chleh Jul 17, 2018

Collaborator

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.

This comment has been minimized.

@wenqing

wenqing Jul 17, 2018

Member

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)

This comment has been minimized.

@wenqing

wenqing Jul 13, 2018

Member

The same question.

config.getConfigParameter<bool>("strict_bc_domain", true);
if (!strict_bc_domain)
{
DBUG("Allow non-strict bc domain and global component mapping.");

This comment has been minimized.

@wenqing

wenqing Jul 13, 2018

Member

bc-->BC?


if (!boundary_mesh)
// Surface mesh and bulk mesh must have equal axial symmetry flags!
if (boundary_mesh.isAxiallySymmetric() != bulk_mesh.isAxiallySymmetric())

This comment has been minimized.

@wenqing

wenqing Jul 13, 2018

Member

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);

This comment has been minimized.

@wenqing

wenqing Jul 13, 2018

Member

Is the comment needed?

@endJunction endJunction force-pushed the endJunction:AllowMultipleMeshesInProject_j branch 3 times, most recently from 0e05fd6 to 97eb96e Jul 16, 2018

@endJunction endJunction force-pushed the endJunction:AllowMultipleMeshesInProject_j branch 6 times, most recently from 8272cc3 to 52269b3 Aug 10, 2018

@endJunction endJunction force-pushed the endJunction:AllowMultipleMeshesInProject_j branch from f56ea8a to 0624ba8 Aug 23, 2018

@endJunction endJunction force-pushed the endJunction:AllowMultipleMeshesInProject_j branch from 0624ba8 to 796ccf3 Aug 23, 2018

@endJunction endJunction force-pushed the endJunction:AllowMultipleMeshesInProject_j branch from 796ccf3 to e894e5c Aug 23, 2018

@endJunction endJunction force-pushed the endJunction:AllowMultipleMeshesInProject_j branch from 37f0ae6 to b1ae94c Aug 23, 2018

@endJunction endJunction removed the WIP 🏗 label Aug 23, 2018

@wenqing
Copy link
Member

wenqing left a comment

👍

@endJunction endJunction merged commit d9eed42 into ufz:master Aug 23, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
deploy/netlify Deploy preview ready!
Details

@endJunction endJunction deleted the endJunction:AllowMultipleMeshesInProject_j branch Aug 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.