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

Created multicomponent EoS->Material averaging function #4148

Merged
merged 2 commits into from
Jul 8, 2021

Conversation

bobmyhill
Copy link
Member

@bobmyhill bobmyhill commented Jul 8, 2021

This PR adds a function that does multicomponent EquationOfState property averaging.
This function is stored in the equation of state interface, in the MaterialModel namespace.

@bobmyhill bobmyhill force-pushed the utility_avg branch 2 times, most recently from 315a997 to 58b6bfe Compare July 8, 2021 16:19
@naliboff
Copy link
Contributor

naliboff commented Jul 8, 2021

@bobmyhill - Awesome! Let me know when you would like a review for this!

@bobmyhill bobmyhill changed the title [WIP] Created multicomponent EoS->Material averaging function Created multicomponent EoS->Material averaging function Jul 8, 2021
@bobmyhill
Copy link
Member Author

Hi @naliboff, this is ready for a review now :)

Copy link
Contributor

@naliboff naliboff left a comment

Choose a reason for hiding this comment

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

@bobmyhill - looks good, thanks for moving this code to a utilities function! Before merging, is your plan to go through the other material models and use this function? I believe this could at least be used in visco_plastic that has nearly identical lines of code?

@bobmyhill
Copy link
Member Author

Hi @naliboff, yes, that's the intention (in another PR)!

Copy link
Contributor

@naliboff naliboff left a comment

Choose a reason for hiding this comment

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

@bobmyhill - Sounds good, let me know when the other PR is ready for review!

@naliboff naliboff merged commit 2aedc7a into geodynamics:master Jul 8, 2021
@jdannberg
Copy link
Contributor

So @bobmyhill I thought we had discussed yesterday that the averaging is the responsibility of the material model, not of the equation of state, so this functionality should be part of the material model utilities, not of the interface of the equation of state. The equation of state only knows things about individual compositions, and the material model decides how the material properties are computed from that. Could you fix that in a follow-up pull request? Or did you have a different reason why you thought that should go into the equation of state rather than in the material model?

@bobmyhill
Copy link
Member Author

@jdannberg I did try to do it the way we discussed, but the include hierarchy seems to be structured in such a way as to make that difficult (the EoS interface knows about the material model utilities/classes, not the other way round). Do you think there's a robust workaround?

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.

3 participants