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

Add max_model_depth variable to cartesian and spherical geometries #367

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- The option for a temperature model for an oceanic plate with a constant age using the plate model. \[Haoyuan Li; 2021-10-29; [#344](https://github.com/GeodynamicWorldBuilder/WorldBuilder/pull/344)\]
- The cmake targets to easily switch between debug and release mode \[Menno Fraters; 2021-10-31; [#361](https://github.com/GeodynamicWorldBuilder/WorldBuilder/pull/361)\]
- A new depth method for the spherical coordinate system called begin at end segment. This adds the spherical correction to the end of the segment instead of the beginning, resulting in a smoother transition between segments. \[Menno Fraters; 2021-11-06; [#364](https://github.com/GeodynamicWorldBuilder/WorldBuilder/pull/364), [#365](https://github.com/GeodynamicWorldBuilder/WorldBuilder/pull/365)\]
- A new input parameter and accociated functions which define the maximum depth of a model. This allows the world builder to create a complete picture of the world. \[Menno Fraters; 2021-11-08; [#367](https://github.com/GeodynamicWorldBuilder/WorldBuilder/pull/367) and [#331](https://github.com/GeodynamicWorldBuilder/WorldBuilder/issues/331)\]

### Changed
- The World Builder Visualizer will now use zlib compression for vtu files by default. If zlib is not available binary output will be used. \[Menno Fraters; 2021-06-26; [#282](github.com/GeodynamicWorldBuilder/WorldBuilder/pull/282)\]
Expand Down
6 changes: 6 additions & 0 deletions include/world_builder/coordinate_systems/cartesian.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@ namespace WorldBuilder
*/
double distance_between_points_at_same_depth(const Point<3> &point_1, const Point<3> &point_2) const override final;

/**
* Returns the max model depth. This always returns infinity for Cartesian
* models.
*/
virtual
double max_model_depth() const override final;

private:

Expand Down
7 changes: 7 additions & 0 deletions include/world_builder/coordinate_systems/interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,13 @@ namespace WorldBuilder
virtual
double distance_between_points_at_same_depth(const Point<3> &point_1, const Point<3> &point_2) const = 0;

/**
* Returns the max model depth. This should be the infinity for Cartesian
* models and the radius in spherical models.
*/
virtual
double max_model_depth() const = 0;

/**
* A function to register a new type. This is part of the automatic
* registration of the object factory.
Expand Down
13 changes: 12 additions & 1 deletion include/world_builder/coordinate_systems/spherical.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,24 @@ namespace WorldBuilder
*/
double distance_between_points_at_same_depth(const Point<3> &point_1, const Point<3> &point_2) const override final;

/**
* Returns the max model depth. This returns the radius in spherical
* models.
*/
virtual
double max_model_depth() const override final;

/**
* What depth method the spherical coordinates use.
*/
DepthMethod used_depth_method;

private:

/**
* The maximum depth of the model. In the spherical case that is equal to the
* radius of the sphere.
*/
double radius_sphere;
};
} // namespace CoordinateSystems
} // namespace WorldBuilder
Expand Down
7 changes: 7 additions & 0 deletions source/coordinate_systems/cartesian.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ namespace WorldBuilder
return point_at_depth.norm();
}


double
Cartesian::max_model_depth() const
{
return std::numeric_limits<double>::infinity();
}
MFraters marked this conversation as resolved.
Show resolved Hide resolved

/**
* Register plugin
*/
Expand Down
16 changes: 14 additions & 2 deletions source/coordinate_systems/spherical.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@


#include "world_builder/types/object.h"
#include "world_builder/types/double.h"
#include "world_builder/utilities.h"

namespace WorldBuilder
Expand All @@ -40,7 +41,7 @@ namespace WorldBuilder
{

// Add depth method to the requried parameters.
prm.declare_entry("", Types::Object({"depth method"}), "Coordinate sysetm object");
prm.declare_entry("", Types::Object({"depth method"}), "Coordinate system object");


prm.declare_entry("depth method",
Expand All @@ -49,6 +50,10 @@ namespace WorldBuilder
R"('begin segment' and 'begin at end segment'. See the manual section on coordinate systems for )"
R"(more info.)");

prm.declare_entry("radius",
Types::Double(6371000.),
R"(The radius of the sphere.)");


}

Expand All @@ -71,7 +76,7 @@ namespace WorldBuilder
"coordinates. The available options are 'starting point', 'begin segment' and 'begin at end segment'. "
"The option 'continuous' is not yet available.");

//std::cout << "string_depth_method = " << string_depth_method << std::endl;
radius_sphere = prm.get<double>("radius");
}
prm.leave_subsection();
}
Expand Down Expand Up @@ -133,6 +138,13 @@ namespace WorldBuilder
return radius * std::atan2(top, bottom);
}


double
Spherical::max_model_depth() const
{
return radius_sphere;
}

/**
* Register plugin
*/
Expand Down
46 changes: 45 additions & 1 deletion source/parameters.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1383,7 +1383,51 @@ namespace WorldBuilder
}
else if (type == "object")
{
collapse += "/properties";
// normal objects just have properties, but some have oneOf
if (Pointer((base_path + "/oneOf").c_str()).Get(declarations) != nullptr)
{
// it has a structure with oneOf. Find out which of the entries is needed.
// This means we have to take a sneak peak to figure out how to get to the
// next value.
size_t size = Pointer((base_path + "/oneOf").c_str()).Get(declarations)->Size();
#ifdef debug
bool found = false;
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has nothing to do with the patch itself, but just as a comment: I assume that you are putting the #ifdef around this statement because you would otherwise get a warning about the found variable only ever being written to, but never read, in release mode? If so, you can avoid the warning if you put

  (void)found;

in front of the WBAssert. This is interpreted as a "read" of the variable, but you are just discarding the value so read.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting, I will try that!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on my laptop those if statement do not seem to matter, but probably for one of the tester configurations it does. I will look into it in a different pull request.

size_t index = 0;
for (; index < size; ++index)
{
std::string declarations_string = Pointer((base_path + "/oneOf/" + std::to_string(index)
+ "/properties/model/enum/0").c_str()).Get(declarations)->GetString();

// we need to get the json path relevant for the current declaration string
// we are interested in, which requires an offset of 2.
WBAssert(Pointer((get_full_json_path(i+2) + "/model").c_str()).Get(parameters) != nullptr, "Could not find model in: " << get_full_json_path(i+2) + "/model");
std::string parameters_string = Pointer((get_full_json_path(i+2) + "/model").c_str()).Get(parameters)->GetString();

// currently in our case these are always objects, so go directly to find the option we need.
if (declarations_string == parameters_string)
{
// found it for index i;
#ifdef debug
found = true;
#endif
break;
}
}
#ifdef debug
WBAssert(found == true,
"Internal error: This is an array with several possible values, "
"but could not find the correct value " << collapse + "/" + path[i] + "/items/oneOf");
#endif
collapse += "/" + path[i] + "/oneOf/" + std::to_string(index) + "/properties";
// add one to i, to skip the array
++i;

}
else
{
collapse += "/properties";
}
}
else
{
Expand Down
7 changes: 6 additions & 1 deletion tests/unit_tests/unit_test_world_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3823,9 +3823,12 @@ TEST_CASE("WorldBuilder Parameters")
{
// First test a world builder file with a cross section defined
std::string file = WorldBuilder::Data::WORLD_BUILDER_SOURCE_DIR + "/tests/data/type_data.json";
std::string file_name = WorldBuilder::Data::WORLD_BUILDER_SOURCE_DIR + "/tests/data/subducting_plate_different_angles_spherical.wb";
std::string file_name = WorldBuilder::Data::WORLD_BUILDER_SOURCE_DIR + "/tests/data/subducting_plate_different_angles_cartesian.wb";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't you abandoning the old test here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed the only thing happening with file_name here is WorldBuilder::World world(file_name); and then Parameters prm(world);. Then the prm is actually initialized with a different file. These first two steps on this exact file are already being tested in line 7099-7100, so there is no reason to repeat that here. And there was no place in the unit tester which did this for the Cartesian case, so I think it is a nice expansion of the testing without losing anything.

I could have made a new test as well, but I guess I was a bit lazy ;)

WorldBuilder::World world(file_name);

world.parse_entries(world.parameters);
CHECK(std::isinf(world.parameters.coordinate_system->max_model_depth()));

Parameters prm(world);
prm.initialize(file);

Expand Down Expand Up @@ -7344,6 +7347,8 @@ TEST_CASE("WorldBuilder Utilities function: distance_point_from_curved_planes sp
WorldBuilder::World world(file_name);

const double dtr = Utilities::const_pi/180.0;
world.parse_entries(world.parameters);
CHECK(world.parameters.coordinate_system->max_model_depth() == Approx(6371000.));
// slab goes down and up again
// origin
std::array<double,3> position = {{6371000 - 0, 0 * dtr, 0 * dtr}};
Expand Down