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

Fix phases for background material and plasticity in the grain size material model. #6269

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gassmoeller
Copy link
Member

The implementation of plasticity in the grain size material model currently has minor issues that arise when one tries to use it together with multiple phase transitions and multiple compositions, because the function drucker_prager_plasticity.compute_drucker_prager_parameters is called with the number of phases instead of the number of phase transitions that it expects (off by one). This leads to triggered asserts no matter how one sets the input parameters.

I added a test that fails on the main branch and works with this change.

@jdannberg can you take a look?

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.

This looks reasonable to me and the other tests still work.
Thank you for fixing this!

Comment on lines +1 to +2
# This test is like grain_size_phase_function.prm, but
# it additionally enables plastic yielding in the material model.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to include the other test? If the only thing you changed is the yielding?

@@ -979,10 +978,11 @@ namespace aspect
phase_function->initialize_simulator (this->get_simulator());
phase_function->parse_parameters (prm);

std::vector<unsigned int> n_phases_for_each_composition = phase_function->n_phases_for_each_composition();
n_phase_transitions = n_phases_for_each_composition[0] - 1;
// Phase transitions are only supported for the background composition.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure that's quite the right way to say it. There is not really a background composition. The phase function is only used for the viscosity, and you can't specify different viscosities for the different compositions. So technically, the phase transitions are the same for all compositions.

@gassmoeller
Copy link
Member Author

Thanks for the review, I addressed your comments, feel free to take another look.

@jdannberg
Copy link
Contributor

Your changes look good to me, but your new test fails now ☹️

@gassmoeller gassmoeller force-pushed the grain_size_multicomposition branch from 27730d1 to 7193986 Compare April 9, 2025 15:14
@gassmoeller
Copy link
Member Author

I think I found it. I forgot to remove the tangential velocity boundary conditions in the reworked test file, so now there were tangential and prescribed boundaries. Let's see if the tester is happy now.

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

Successfully merging this pull request may close these issues.

2 participants