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

Move morency doin material model to cookbook #2005

Merged
merged 1 commit into from Nov 21, 2017

Conversation

gassmoeller
Copy link
Member

Something I wanted to do for a long time. Move the Morency & Doin material model into a separate folder in the cookbooks, and remove it from the other material models. Also shorten the manual section by removing unnecessary parameters.

@bangerth
Copy link
Contributor

Hm, the morency_doin test fails. Can you take a look?

As for the actual content of the patch, could someone with more knowledge of the material models take a look?

@gassmoeller
Copy link
Member Author

Yes, the test now has a line about loading the shared library.

I think @jperryhouts was the only one working with this material model. Jonathan, could you have a look at this? It is really just moving two files and some lines around (and shortening the manual somewhat).

@jperryhouts
Copy link
Contributor

I am all for this! I implemented this to reproduce a paper once, and haven't found it useful since. I even suggested removing it all together a while ago.

Also, does anyone know of a way to get more obvious notifications of @ mentions? As it is I'm 'watching' this repository so I get emails about all these comments anyway. Who knows how many threads I've accidentally ignored because of that!

@jdannberg
Copy link
Contributor

@jperryhouts
I was curious about the @ mentions to, and I found this: If you were mentioned in a post, in the cc of the email mention@noreply.github.com will appear. So you can filter your emails using that.

Copy link
Contributor

@jdannberg jdannberg left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this!
I guess there's not much to comment here as you really mostly moved the files.

@@ -0,0 +1,39 @@
# Copyright (C) 2011 - 2016 by the authors of the ASPECT code.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright year?

@@ -23,5 +23,5 @@ set xlabel "Viscosity (Pa s)"
set logscale x
set xrange [1e18:1e28]
set xtics 1e18,100,1e30
plot "< grep -v -e \"^$\" -e \" 0 $\" output/depth_average.gnuplot" using 11:(-$1/1000) with lines
plot "< grep -v -e \"^$\" -e \" 0 $\" output/depth_average.gnuplot" using 14:(-$1/1000) with lines
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this line change? I guess you checked that it produces the correct figure with this version?

@gassmoeller
Copy link
Member Author

I fixed the copyright years. The different column is a consequence of our changes to the statistics file over time (likely the addition of the time step length, possibly the nonlinear iteration count, and or Stokes inner iterations). I checked that the figure now produces the same result as is included in the manual.

@gassmoeller gassmoeller merged commit b7dc686 into geodynamics:master Nov 21, 2017
@gassmoeller gassmoeller deleted the move_morency_doin branch November 21, 2017 21:34
@jperryhouts
Copy link
Contributor

@jdannberg Thanks! That's super helpful. It looks like there's also an undocumented header,
"X-GitHub-Reason: mention", which can be filtered too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants