-
Notifications
You must be signed in to change notification settings - Fork 60
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
Add heat flux postprocessor #953
Conversation
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 minor comments, good work! After my suggestions it is approved
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.
Minor minor changes, but please add a bit more comments
thermal_conductivity_models; | ||
std::vector<HeatFluxPostprocessor<dim>> heat_flux_postprocessors; |
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.
Do we need to recreate those or we could just get them from the physical property manager?
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.
Since its a private member, we have to make a copy... However, I can simply initialize it with values in the constructor instead of in attach_solution_to_output
. I'll do this change!
include/solvers/postprocessors.h
Outdated
HeatFluxPostprocessor( | ||
std::shared_ptr<ThermalConductivityModel> p_thermal_conductivity_model, | ||
const std::string p_material_string = "f", | ||
const unsigned int p_material_id = 0, | ||
const unsigned int p_id = 0) | ||
: DataPostprocessorVector<dim>("heat_flux_" + p_material_string + | ||
Utilities::to_string(p_id, 2), | ||
update_values | update_gradients) | ||
, thermal_conductivity_model(p_thermal_conductivity_model) | ||
, material_id(p_material_id) | ||
{} |
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.
Can you explain a bit more why you need to pass the p_id and the p_material id. It's not clear at this stage
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.
-
p_id
is the number of either the fluid or the solid. So when, for example, we have a 1 fluid and 1 solid, they both have a p_id of0
(fluid_id = 0
andsolid_id = 0
). -
The
material_id
is the cumulative id of fluids and solids (a more global id that's give). From what I've seen fluids are counted first followed by solids. Therefore, in our examples, the fluid would have amaterial_id
of0
and the solid of1
.
The material_id
is used to which cells belong in which material. And p_id
is used for the name of the outputted vector. I could've done it with the a if
and the p_material_string
, but I wanted to avoid the if
.
I'll try to make it more clear. :)
Also, I just realized that the arguments are not entered by reference, I'll change that!
include/solvers/postprocessors.h
Outdated
Vector<double> null_vector(dim); | ||
std::vector<Vector<double>> null_computed_quantities(n_quadrature_points, |
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.
What are those null? Can you comment out why they are required and what's their point?
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.
It's for when the cell doesn't lie in the right material, to avoid doing a for
loop to insert zeros. I'll move it down, where the comment is already there and to avoid making this null-vector for nothing.
I'll also add more comments in general. :)
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.
Nice addition! I agree with Bruno that a bit more comments would help :) Also, may be you can add a test to verify your implementation? I don't know if it is relevant at this point, but a pseudo 1D case with different layers of different materials could be interesting (there are analytical solutions for that in the BSL I think!)
, material_id(p_material_id) | ||
{} | ||
|
||
virtual void |
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 noticed that every overriden methods in postprocessors.h are virtual. Are we expecting to further override them? It is more for my own knowledge, because I'm not sure I get why we do this :)
6db8a49
to
920e2a4
Compare
Since it the values can only be read from an output file, a simple test cannot be made without postprocessing the data in the exported files. However, an example is on the way! |
920e2a4
to
58143f4
Compare
Co-authored-by: Amishga Alphonius <[email protected]>
Co-authored-by: Olivier Guévremont <[email protected]>
Co-authored-by: Bruno Blais <[email protected]>
58143f4
to
6abe21d
Compare
Description of the problem In the context of conjugated heat transfer simulations, there was no way to get the local heat fluxes of the domain. Description of the solution A new DataPostprocessorVector child class was made, namely, HeatFluxPostprocessor. This class postprocesses the local heat fluxes of the fluids and solids of the domain. The values for each fluid and solid is outputed individually. How Has This Been Tested? CI Comments An example using this HeatFluxPostprocessor values will be made soon. Co-authored-by: Amishga Alphonius <[email protected]> Co-authored-by: Olivier Guévremont <[email protected]> Co-authored-by: Amishga Alphonius <[email protected]> Co-authored-by: Bruno Blais <[email protected]> Former-commit-id: d9b9d74
Description of the problem In the context of conjugated heat transfer simulations, there was no way to get the local heat fluxes of the domain. Description of the solution A new DataPostprocessorVector child class was made, namely, HeatFluxPostprocessor. This class postprocesses the local heat fluxes of the fluids and solids of the domain. The values for each fluid and solid is outputed individually. How Has This Been Tested? CI Comments An example using this HeatFluxPostprocessor values will be made soon. Co-authored-by: Amishga Alphonius <[email protected]> Co-authored-by: Olivier Guévremont <[email protected]> Co-authored-by: Amishga Alphonius <[email protected]> Co-authored-by: Bruno Blais <[email protected]> Former-commit-id: d9b9d74
Description of the problem
Description of the solution
DataPostprocessorVector
child class was made, namely,HeatFluxPostprocessor
. This class postprocesses the local heat fluxes of the fluids and solids of the domain. The values for each fluid and solid is outputed individually.How Has This Been Tested?
Comments
An example using this HeatFluxPostprocessor values will be made soon.