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

[WIP] Fix density used for thermal conductivity in visco_plastic and diffusion_dislocation models #2170

Merged
merged 2 commits into from Apr 27, 2018

Conversation

naliboff
Copy link
Contributor

In the visco_plastic and diffusion_dislocation models the thermal diffusivity is specified as an input file parameter instead of the thermal conductivity. The thermal conductivity is then calculated from the thermal diffusivity, density and heat capacity.

This leads to an inconsistency when set Temperature equation = reference density profile.

The same inconsistency is present in any heating model that uses the material model density output. @gassmoeller @jdannberg - I was planning to use the structure to enforce that the heating models use the correct density. Any objections before I do this in another commit?

Thanks and credit to Phillip Heron and Jeroen van Hunen for identifying an issue by running the Blankenbach benchmarks with these two material models!

@jdannberg
Copy link
Contributor

The same inconsistency is present in any heating model that uses the material model density output.

This should already be implemented correctly. If you look in assembly.cc, around line 859, if the formulation for the temperature equation is reference_density_profile, then the heating models get the reference density as an input instead of the real density.

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 fixing this! Your changes look good, but I've found a few more places where the diffusivity is computed, and it would be great if you could fix those too (otherwise, let me know and I'll merge this pull request and we can open another pull request for these new fixes):

  • the visualization postprocessor thermal_diffusivity.cc
  • in the basic_statistics postprocessor in line 75
  • in the initial temperature plugin adiabatic (line 92)

If we want to be really consistent, we could also use the reference density when computing the conduction time step, but I think we can leave that for now.

The changes in these other files would probably be very similar to the ones you did here, so I hope that's not too much work.

@naliboff
Copy link
Contributor Author

@jdannberg - Thanks of for the review and glad that the heating model formulation is already implemented correctly. Happy to fix the diffusivity in the other files you listed as well ... seems logical to do this in the same PR.

@naliboff
Copy link
Contributor Author

@jdannberg - I've fixed the diffusivity in all the places above. Let me know if you would like further documentation or reformatting in these sections.

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.

Looks good, thank you!

@jdannberg jdannberg merged commit 89e76ee into geodynamics:master Apr 27, 2018
@naliboff naliboff deleted the fix_material_model_density branch June 8, 2018 19:27
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

2 participants