Geometry checking and including spherical models #1698
Conversation
|
/run-tests |
bobmyhill
force-pushed
the
geometry_checking
branch
from
97cc462
to
cb64cfd
Compare
| @@ -87,6 +89,7 @@ namespace aspect | |||
| /** | |||
| * Compositions at the inner and outer boundaries. | |||
| */ | |||
| double surface_composition; | |||
| double inner_composition; | |||
There was a problem hiding this comment.
what does the new variable do? the class already has inner/outer boundary values, and the comment doesn't help either :-)
| if (dynamic_cast<const GeometryModel::Sphere<dim>*>(geometry_model) != 0) | ||
| { | ||
| if (boundary_name == "surface") | ||
| return surface_composition; |
There was a problem hiding this comment.
I think I would prefer if this returned "outer_composition", and that we document this in the definition of the input parameters
| @@ -99,6 +120,9 @@ namespace aspect | |||
| { | |||
| prm.enter_subsection("Spherical constant"); | |||
| { | |||
| prm.declare_entry ("Surface composition", "0", | |||
| Patterns::Double (), | |||
| "Composition at the surface (lithosphere water/air). Units: none."); | |||
There was a problem hiding this comment.
my proposal would remove this parameter, and add a sentence to the definition of "Outer composition" to say that that is used for the surface when the geometry is a sphere.
| // because the domain can be non-coordinate parallel. | ||
|
|
||
| AssertThrow(gm->get_eccentricity() == 0.0, | ||
| ExcMessage("This plugin cannot be used with a non-zero eccentricity. ")); |
There was a problem hiding this comment.
This sounds more like you want to use ExcNotImplemented() given the TODO above
| else | ||
| { | ||
| Assert (false, ExcMessage ("This initial condition can only be used if the geometry " | ||
| "is a sphere, a spherical shell or a chunk.")); |
| // because the domain can be non-coordinate parallel. | ||
|
|
||
| AssertThrow(gm->get_eccentricity() == 0.0, | ||
| ExcMessage("This plugin cannot be used with a non-zero eccentricity. ")); |
| "is a sphere, a spherical shell or a chunk.")); | ||
|
|
||
| R0 = std::numeric_limits<double>::quiet_NaN(); | ||
| R1 = std::numeric_limits<double>::quiet_NaN(); |
There was a problem hiding this comment.
That's one way. You shouldn't get here, so it doesn't matter, but we've started using numbers::signaling_nan<double>() in such cases -- that really makes sure that nobody uses these values!
| // for this geometry do we know for sure what boundary indicators it | ||
| // uses and what they mean | ||
| Assert (dynamic_cast<const GeometryModel::Box<dim>*>(&this->get_geometry_model()) | ||
| != 0, | ||
| ExcMessage ("This boundary model is only implemented if the geometry is " | ||
| "in fact a time_dep_box.")); | ||
| "a time_dep_box.")); |
There was a problem hiding this comment.
You really didn't like the "in fact" in all of these places, did you? ;-)
There was a problem hiding this comment.
It's a personal bugbear, in fact!
bobmyhill
force-pushed
the
geometry_checking
branch
2 times, most recently
from
ad40ab6
to
f3b5d90
Compare
|
One of the tests also fails. Do you know why? |
|
Yes, this will be because I didn't have the test output until amending the "updated tests" commit. |
bobmyhill
force-pushed
the
geometry_checking
branch
from
f3b5d90
to
2433487
Compare
|
OK to merge. |
bobmyhill
force-pushed
the
geometry_checking
branch
3 times, most recently
from
d218410
to
c8dfbe8
Compare
bobmyhill
force-pushed
the
geometry_checking
branch
from
c8dfbe8
to
d17c25c
Compare
This is a continuation of PR #1685. Additionally, the included GeometryModels in several files were inconsistent; this PR adds Sphere to boundary_composition/spherical_constant.cc and EllipsoidalChunk to initial_temperature/spherical_shell.cc. Tests are created for both of these new cases.