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

Output dynamic topography as point-wise vector #2791

Merged

Conversation

Projects
None yet
3 participants
@gassmoeller
Copy link
Contributor

commented Jan 30, 2019

This addresses the dynamic topography part of #2788 and was really simpler than I expected. Even without using interpolated visualization output the effect is nice (side-by-side comparison of the S20RTS cookbook with and without the new output):

s20rts_dynamic_topo_comparison

@gassmoeller gassmoeller force-pushed the gassmoeller:dynamic_topography_point_wise_vector branch from 84abdbe to dedea95 Jan 30, 2019

@MFraters
Copy link
Contributor

left a comment

Looks great Rene! I just have some comments on documentation of the code.

@@ -44,10 +44,13 @@ namespace aspect
*/

This comment has been minimized.

Copy link
@MFraters

MFraters Feb 28, 2019

Contributor

Please update the class description

@@ -66,8 +69,9 @@ namespace aspect
* to.

This comment has been minimized.

Copy link
@MFraters

MFraters Feb 28, 2019

Contributor

Same here.

new Vector<float>(dynamic_topography.cellwise_topography()));
auto cell = input_data.template get_cell<DoFHandler<dim> >();

bool cell_at_top_or_bottom_boundary = false;

This comment has been minimized.

Copy link
@MFraters

MFraters Feb 28, 2019

Contributor

Maybe the code could use some more comment to make reading it easier. You could for example put a comment here like: "We are only interested in the faces at the top and bottom of the domain, so we check that here."


if (cell_at_top_or_bottom_boundary)
{
std::vector<Point<dim>> quadrature_points(input_data.evaluation_points.size());

This comment has been minimized.

Copy link
@MFraters

MFraters Feb 28, 2019

Contributor

And here something like: "This cell is at the top or bottom, now compute the values of the dynamic topography in this cell."
It might be just my personal style, but I like very verbose comments. If you feel it clutters the code too much, feel free to leave it out.

fe_volume_values.reinit(cell);

std::vector<double> dynamic_topography_values(quadrature_formula.size());
fe_volume_values[this->introspection().extractors.temperature].get_function_values(dynamic_topography.topography_vector(),

This comment has been minimized.

Copy link
@naliboff

naliboff Feb 28, 2019

Contributor

Might be worth putting a short comment here explaining why one needs to use the temperature quadrature.

@gassmoeller gassmoeller force-pushed the gassmoeller:dynamic_topography_point_wise_vector branch from 2365bb8 to 9ec9a95 Mar 1, 2019

@gassmoeller

This comment has been minimized.

Copy link
Contributor Author

commented Mar 1, 2019

I addressed your comments.

@MFraters MFraters merged commit 1e803e0 into geodynamics:master Mar 1, 2019

2 checks passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins.tjhei.info/pr-head This commit looks good
Details

@gassmoeller gassmoeller deleted the gassmoeller:dynamic_topography_point_wise_vector branch Mar 1, 2019

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.