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
Multicomponent equation of state #3075
Conversation
a2a6585
to
1cbf07f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks pretty obvious, but it's a lot of code churn. If #3036 is merged, please rebase and then let's see whether the tester is happy with the result.
1cbf07f
to
873090f
Compare
This is now rebased to the current master. |
There are two failing tests:
|
I updated the test results. The only difference was a change in iteration count. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice. Please address my documentation comments, and then this is ready.
|
||
/** | ||
* Read the parameters this class declares from the parameter file. | ||
* The optional parameter @p n_compositions determines the maximum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no optional parameter here.
|
||
/** | ||
* Declare the parameters this class takes through input files. | ||
* The optional parameter @p n_compositions determines the maximum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a different optional parameter here.
|
||
|
||
private: | ||
// Vector of reference densities $\rho_0$ with one entry per composition, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make these comments into doxygen documentation: /** ... */. Same for all parameters below.
has_background_field, | ||
"Specific heats"); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove empty line
I addressed your comments. |
This is another pull request based on #3036 (so only the last commit is new).
It adds an equation of state for multicomponent models and uses it in 4 different material models. There is another material model where this could potentially be used, which is the diffusion_dislocation model. The difference here is that diffusion_dislocation only has one value for the specific heat (but a vector with entries for each composition for the densities and thermal expansivities). This may be worth changing.
What do you think?