Allowing multiple traction models for a boundary #5220

chameerasilva · 2023-07-08T22:10:02Z

No description provided.

gassmoeller

This goes into the right direction. I have just left some large-scale comments for now, I will take a closer look when you have addressed these comments. Let me know when you have addressed them.

gassmoeller · 2023-07-08T22:28:37Z

source/boundary_traction/interface.cc

+// ------------------------------ Manager -----------------------------
+// -------------------------------- Deal with registering boundary_traction models and automating
+// -------------------------------- their setup and selection at run time
+// just copy and paste mangaer class from boundary-velocity/interface.cc from line 66 -428  {need to change the descriptions....}


This type of documentation should be in the header file following the same format as the other documentation strings there.

gassmoeller · 2023-07-08T22:29:42Z

source/boundary_traction/interface.cc

+                           "may have provided for each part of the boundary. You may want "
+                           "to compare this with the documentation of the geometry model you "
+                           "use in your model.");
+        // prm.declare_entry ("Tangential velocity boundary indicators", "",


One of our conventions is to never leave commented code in the repository. In the future it will not be possible to determine why this code is commented. Is it work in progress? Is it outdated? Is it wrong?
We dont need this code, so just remove the whole commented section. Here, and in all other places of your changes.

gassmoeller · 2023-07-08T22:32:05Z

include/aspect/boundary_traction/interface.h

+         * A function that is called at the beginning of each time step and
+         * calls the corresponding functions of all created plugins.
+         *
+         * The point of this function is to allow complex boundary velocity


You still reference the velocity in many places of the header and the source file. The simplest way to fix this is to search for velocity and replace it with traction in all places you find it (you can use ctrl+f in VS code to do this quite quick).

Suggested change

* The point of this function is to allow complex boundary velocity

* The point of this function is to allow complex boundary traction

gassmoeller · 2023-07-08T22:34:19Z

source/boundary_traction/interface.cc

+// explicit instantiations
+namespace aspect
+{
+  namespace internal
+  {
+    namespace Plugins
+    {
+      template <>
+      std::list<internal::Plugins::PluginList<BoundaryTraction::Interface<2>>::PluginInfo> *
+      internal::Plugins::PluginList<BoundaryTraction::Interface<2>>::plugins = nullptr;
+      template <>
+      std::list<internal::Plugins::PluginList<BoundaryTraction::Interface<3>>::PluginInfo> *
+      internal::Plugins::PluginList<BoundaryTraction::Interface<3>>::plugins = nullptr;
+    }
+  }
+
+  namespace BoundaryTraction
+  {
+#define INSTANTIATE(dim) \
+  template class Interface<dim>; \
+  template class Manager<dim>;
+
+    ASPECT_INSTANTIATE(INSTANTIATE)
+
+#undef INSTANTIATE


There is already a section further down in this file labeled #explicit instantiations. Give it a try to merge your changes here with that section further down and find me if you run into problems.

gassmoeller

A few more comments. After you have addressed them, find me and we can talk about how to change the core of ASPECT (the Simulator class) to use the new manager class.

gassmoeller · 2023-07-09T03:59:33Z

include/aspect/boundary_traction/interface.h

@@ -125,7 +125,181 @@ namespace aspect
         */
        const GeometryModel::Interface<dim> *geometry_model;
    };
+    template <int dim>


Please add one empty line before the class. Our usual convention is to have one empty line between functions/classes in the header file and three empty lines between functions in the source .cc file.

gassmoeller · 2023-07-09T04:02:43Z

include/aspect/boundary_traction/interface.h

+         * to combine them.
+         */
+        Tensor<1,dim>
+        boundary_traction (const types::boundary_id boundary_indicator,


Here you will have to change something compared to the velocity manager. Take a look at line 100 of this file, the boundary_traction function has a slightly different interface. In particular it takes a third argument that indicates the normal direction on the current boundary. You will have to add this here as an additional argument as well, otherwise the manager cannot call the respective function of the individual plugins.

gassmoeller · 2023-07-09T04:20:22Z

include/aspect/boundary_traction/interface.h

+         */
+        static
+        void
+        register_boundary_traction (const std::string &name,


Now that you have this function inside the manager class you can remove the other register_boundary_traction function below (and the function implementation in the source file).

gassmoeller · 2023-07-09T04:21:48Z

include/aspect/boundary_traction/interface.h

+         * Return a list of boundary ids on which the traction is prescribed
+         * to be zero (no-slip).
+         */
+        const std::set<types::boundary_id> &
+        get_zero_boundary_traction_indicators () const;


There is nothing like this for the boundary traction (a zero boundary traction plugin just returns a traction of 0, it is not in any way special). Remove this function here and in the source file.

gassmoeller · 2023-07-09T04:22:29Z

include/aspect/boundary_traction/interface.h

+        /**
+         * A set of boundary indicators, on which velocities are prescribed to
+         * zero (no-slip).
+         */
+        std::set<types::boundary_id> zero_traction_boundary_indicators;


No need to keep this variable.

gassmoeller · 2023-07-09T04:26:33Z

source/boundary_traction/interface.cc

+                           "Note that the no-slip boundary condition is "
+                           "a special case of the current one where the prescribed traction "
+                           "happens to be zero. It can thus be implemented by indicating that "
+                           "a particular boundary is part of the ones selected "
+                           "using the current parameter and using ``zero traction'' as "
+                           "the boundary values. Alternatively, you can simply list the "
+                           "part of the boundary on which the traction is to be zero with "
+                           "the parameter ``Zero traction boundary indicator'' in the "
+                           "current parameter section."


This part does not apply anymore.

gassmoeller · 2023-07-09T04:27:23Z

source/boundary_traction/interface.cc

+                           "Note that when ``Use years in output instead of seconds'' is set "
+                           "to true, traction should be given in m/yr. "


This needs to be updated to the unit of tractions (N/m^2)

gassmoeller · 2023-07-09T04:27:36Z

source/boundary_traction/interface.cc

+        prm.declare_entry ("Zero traction boundary indicators", "",
+                           Patterns::List (Patterns::Anything()),
+                           "A comma separated list of names denoting those boundaries "
+                           "on which the traction is zero."
+                           "\n\n"
+                           "The names of the boundaries listed here can either by "
+                           "numbers (in which case they correspond to the numerical "
+                           "boundary indicators assigned by the geometry object), or they "
+                           "can correspond to any of the symbolic names the geometry object "
+                           "may have provided for each part of the boundary. You may want "
+                           "to compare this with the documentation of the geometry model you "
+                           "use in your model.");


remove this here, and in the parse_parameters function below.

gassmoeller · 2023-07-10T22:35:24Z

@chameerasilva I fixed some of the remaining complicated compiler messages. There are still some more left for you to work on. You can pull the changes of this branch from github into your local branch. Find me if you need help with that.

gassmoeller

Thank you this looks quite good already. I left a lot of small comments, mostly related to code style. Please address these. Also because we are removing a function that was available for all users before, we should add an entry into the ASPECT changelog (see https://aspect.geodynamics.org/doc/doxygen/changes_current.html). Take a look at the directory doc/modules/changes, copy one of the files in there, replace the names, dates, and content with a description that describes what we did in this pull request.

gassmoeller · 2023-07-13T17:06:39Z

include/aspect/boundary_traction/interface.h

+                        << "Could not find entry <"
+                        << arg1
+                        << "> among the names of registered boundary traction objects.");
+        /**


Add an empty line here:

Suggested change

/**

/**

gassmoeller · 2023-07-13T17:07:44Z

include/aspect/boundary_traction/interface.h

+        /**
+         * A function that is used to register boundary traction objects in such
+         * a way that the Manager can deal with all of them without having to
+         * know them by name. This allows the files in which individual
+         * plugins are implemented to register these plugins, rather than also
+         * having to modify the Manager class by adding the new boundary
+         * traction plugin class.
+         *
+         * @param name A string that identifies the boundary traction model
+         * @param description A text description of what this model does and that
+         * will be listed in the documentation of the parameter file.
+         * @param declare_parameters_function A pointer to a function that can be
+         * used to declare the parameters that this boundary traction model
+         * wants to read from input files.
+         * @param factory_function A pointer to a function that can create an
+         * object of this boundary traction model.
+         */


This documentation does not belong to this function. Delete it and copy over the documentation of the same function from the boundary velocity manager and adjust for traction boundaries.

gassmoeller · 2023-07-13T17:10:47Z

source/boundary_traction/ascii_data.cc

-        if (bv.second.get() == this)
-          boundary_ids.insert(bv.first);
-
+      for (const auto &bv : this->get_boundary_traction_manager ().get_active_boundary_traction_conditions ())


remove the spaces between function name and parentheses:

Suggested change

for (const auto &bv : this->get_boundary_traction_manager ().get_active_boundary_traction_conditions ())

for (const auto &bv : this->get_boundary_traction_manager().get_active_boundary_traction_conditions())

gassmoeller · 2023-07-13T17:11:34Z

source/boundary_traction/initial_lithostatic_pressure.cc

@@ -47,10 +47,11 @@ namespace aspect
      // Ensure the initial lithostatic pressure traction boundary conditions are used,
      // and register for which boundary indicators these conditions are set.
      std::set<types::boundary_id> traction_bi;
-      for (const auto &p : this->get_boundary_traction())
+      for (const auto &p : this->get_boundary_traction_manager ().get_active_boundary_traction_conditions ())


as above:

Suggested change

for (const auto &p : this->get_boundary_traction_manager ().get_active_boundary_traction_conditions ())

for (const auto &p : this->get_boundary_traction_manager().get_active_boundary_traction_conditions())

gassmoeller · 2023-07-13T17:12:18Z

source/boundary_traction/interface.cc

@@ -60,9 +60,22 @@ namespace aspect
    Interface<dim>::parse_parameters (dealii::ParameterHandler &)
    {}

+    template <int dim>


We use three empty lines between functions in .cc files (one empty line in .h files):

Suggested change

template <int dim>

template <int dim>

gassmoeller · 2023-07-13T17:29:10Z

source/simulator/melt.cc

          .find (face->boundary_id())
          !=
-          this->get_boundary_traction().end())
+          this->get_boundary_traction_manager ().get_active_boundary_traction_names().end())


Suggested change

this->get_boundary_traction_manager ().get_active_boundary_traction_names().end())

this->get_boundary_traction_manager().get_active_boundary_traction_names().end())

gassmoeller · 2023-07-13T17:29:21Z

source/simulator/melt.cc

-                  ->boundary_traction (face->boundary_id(),
-                                       scratch.face_finite_element_values.quadrature_point(q),
-                                       scratch.face_finite_element_values.normal_vector(q));
+                = this->get_boundary_traction_manager ().


Suggested change

= this->get_boundary_traction_manager ().

= this->get_boundary_traction_manager().

gassmoeller · 2023-07-13T17:29:38Z

source/simulator/melt.cc

@@ -1678,7 +1676,7 @@ namespace aspect
      std::make_unique<aspect::Assemblers::MeltStokesSystemBoundary<dim>>());

    // add the terms for traction boundary conditions
-    if (!this->get_boundary_traction().empty())
+    if (!this->get_boundary_traction_manager ().get_active_boundary_traction_names().empty())


Suggested change

if (!this->get_boundary_traction_manager ().get_active_boundary_traction_names().empty())

if (!this->get_boundary_traction_manager().get_active_boundary_traction_names().empty())

gassmoeller · 2023-07-13T17:29:56Z

source/simulator/newton.cc

@@ -98,7 +98,7 @@ namespace aspect
                             " defined that handles this formulation."));

    // add the terms for traction boundary conditions
-    if (!this->get_boundary_traction().empty())
+    if (!this->get_boundary_traction_manager ().get_active_boundary_traction_names ().empty())


Suggested change

if (!this->get_boundary_traction_manager ().get_active_boundary_traction_names ().empty())

if (!this->get_boundary_traction_manager().get_active_boundary_traction_names().empty())

gassmoeller · 2023-07-13T17:32:30Z

tests/traction_multiple_model.prm

+subsection Boundary traction model
+  set Prescribed traction boundary indicators = 0:ascii data, 0:function
+end
+
+subsection Boundary traction model
+   subsection Function
+    set Variable names      = x,y
+
+    # The following line is the stress applied to the side boundaries (in Pa)
+    set Function constants  = stress=2000e6
+    set Function expression = if (y>600e3 ,-stress,0); 0
+  end
+end
+subsection Boundary velocity model
+  set Tangential velocity boundary indicators = 1,2,3
+end
+
+subsection Boundary traction model
+  subsection Ascii data model
+    set Data directory =$ASPECT_SOURCE_DIR/data/boundary-traction/ascii-data/test/
+  end


You have three occurrences of subsection Boundary traction model here. Please merge all of them into one subsection (one subsection Boundary traction model .... end block).

gassmoeller reviewed Jul 8, 2023

View reviewed changes

gassmoeller reviewed Jul 9, 2023

View reviewed changes

gassmoeller added the ready to test label Jul 13, 2023

gassmoeller reviewed Jul 13, 2023

View reviewed changes

Allowing multiple traction models for a boundary

ef430a3

chameerasilva force-pushed the multiple_traction_models branch from a0043f9 to ef430a3 Compare July 14, 2023 00:10

gassmoeller added the ready to merge label Jul 14, 2023

gassmoeller approved these changes Jul 14, 2023

View reviewed changes

gassmoeller merged commit d9b5b4c into geodynamics:main Jul 14, 2023
6 checks passed

gassmoeller mentioned this pull request Jul 14, 2023

Extend changelog entry #5311

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allowing multiple traction models for a boundary #5220

Allowing multiple traction models for a boundary #5220

chameerasilva commented Jul 8, 2023

gassmoeller left a comment

gassmoeller Jul 8, 2023

gassmoeller Jul 8, 2023

gassmoeller Jul 8, 2023

gassmoeller Jul 8, 2023

gassmoeller left a comment

gassmoeller Jul 9, 2023

gassmoeller Jul 9, 2023

gassmoeller Jul 9, 2023

gassmoeller Jul 9, 2023

gassmoeller Jul 9, 2023

gassmoeller Jul 9, 2023

gassmoeller Jul 9, 2023

gassmoeller Jul 9, 2023

gassmoeller commented Jul 10, 2023

gassmoeller left a comment

gassmoeller Jul 13, 2023

gassmoeller Jul 13, 2023

gassmoeller Jul 13, 2023

gassmoeller Jul 13, 2023

gassmoeller Jul 13, 2023

gassmoeller Jul 13, 2023

gassmoeller Jul 13, 2023

gassmoeller Jul 13, 2023

gassmoeller Jul 13, 2023

gassmoeller Jul 13, 2023

	* The point of this function is to allow complex boundary velocity
	* The point of this function is to allow complex boundary traction

		"Note that when ``Use years in output instead of seconds'' is set "
		"to true, traction should be given in m/yr. "

	for (const auto &bv : this->get_boundary_traction_manager ().get_active_boundary_traction_conditions ())
	for (const auto &bv : this->get_boundary_traction_manager().get_active_boundary_traction_conditions())

	for (const auto &p : this->get_boundary_traction_manager ().get_active_boundary_traction_conditions ())
	for (const auto &p : this->get_boundary_traction_manager().get_active_boundary_traction_conditions())

	this->get_boundary_traction_manager ().get_active_boundary_traction_names().end())
	this->get_boundary_traction_manager().get_active_boundary_traction_names().end())

	= this->get_boundary_traction_manager ().
	= this->get_boundary_traction_manager().

	if (!this->get_boundary_traction_manager ().get_active_boundary_traction_names().empty())
	if (!this->get_boundary_traction_manager().get_active_boundary_traction_names().empty())

Allowing multiple traction models for a boundary #5220

Allowing multiple traction models for a boundary #5220

Conversation

chameerasilva commented Jul 8, 2023

gassmoeller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gassmoeller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gassmoeller commented Jul 10, 2023

gassmoeller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment