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

Flow rates postprocessing #1092

Merged
merged 4 commits into from
Apr 18, 2024
Merged

Flow rates postprocessing #1092

merged 4 commits into from
Apr 18, 2024

Conversation

PierreLaurentinCS
Copy link
Collaborator

Description of the problem

  • The previous flow rate post processing tool would add a line to the .dat file for each defined boundary (in the boundary condition subsection of the .prm file) for every time iteration. This is not the expected output as one would expect a single line to be added for each time step with the flow rates of every boundary.
  • Also, the values of the flow rates were added with a 12 digits precision. Instead, a 4 digits precision in scientific notation was chosen, which seems like a reasonable precision.

Description of the solution

  • The value of time is now added outside of the loop on the boundary conditions.

Comments

  • At the moment there is no unified output format : with/without scientific notation, different numbers of digits. This could be the work of a future PR.

Copy link
Contributor

@blaisb blaisb left a comment

Choose a reason for hiding this comment

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

A few comments regarding formatting.

@@ -1424,13 +1437,13 @@ NavierStokesBase<dim, VectorType, DofsType>::postprocess_fd(bool firstIter)
simulation_parameters.post_processing.flow_rate_output_name +
".dat";
std::ofstream output(filename.c_str());
flow_rate_table.set_precision("time", 12);
flow_rate_table.set_precision("time", 4);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this is a good idea. For the other variables we use 12 significant digits. Since this is always written to a data file, I think it's a much better idea to keep as many digits as possible. Using scientific notation is a good idea however.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I misunderstood. Here is it the one you are printing on the screen or the one your are printing on the data file? For the screen on e I think 4 digit scientific is ok. For the data file one, I think it's not a good idea to print just 4 digits.

Copy link
Contributor

Choose a reason for hiding this comment

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

For log outputted to the terminal you can also youse simulation_control->get_log_precision() which is meant exactly for that.

Copy link
Collaborator Author

@PierreLaurentinCS PierreLaurentinCS Apr 16, 2024

Choose a reason for hiding this comment

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

Agreed, will keep the scientific notation but with 12 digits for the data file. For the screen display I'll keep the way it is coded now which uses the get_log_precision() method

@@ -1424,13 +1437,13 @@ NavierStokesBase<dim, VectorType, DofsType>::postprocess_fd(bool firstIter)
simulation_parameters.post_processing.flow_rate_output_name +
".dat";
std::ofstream output(filename.c_str());
flow_rate_table.set_precision("time", 12);
flow_rate_table.set_precision("time", 4);
for (unsigned int boundary_id = 0;
boundary_id < simulation_parameters.boundary_conditions.size;
++boundary_id)
flow_rate_table.set_precision("flow-rate-" +
std::to_string(boundary_id),
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Utilities::int_to_string instead
and set the padding manually to 2, because we will never have more than 99 boundary conditions (it makes no sense)

@blaisb
Copy link
Contributor

blaisb commented Apr 16, 2024

@PierreLaurentinCS please also suggest two other reviewers

@PierreLaurentinCS PierreLaurentinCS force-pushed the flow_rates_postprocessing branch from 22d153f to 8373fff Compare April 16, 2024 14:35
@PierreLaurentinCS PierreLaurentinCS added Bug Something isn't working Quick fix Needs more reviewers This pull request needs more review before a merge is possible labels Apr 16, 2024
@blaisb
Copy link
Contributor

blaisb commented Apr 16, 2024

@PierreLaurentinCS Can you rebase for the changelog? Then I will be able to merge

@PierreLaurentinCS PierreLaurentinCS force-pushed the flow_rates_postprocessing branch from 4fd57ea to ea65307 Compare April 16, 2024 21:19
@PierreLaurentinCS PierreLaurentinCS added Reviewed and ready to merge and removed Needs more reviewers This pull request needs more review before a merge is possible labels Apr 16, 2024
@blaisb blaisb merged commit 79f3d07 into master Apr 18, 2024
8 checks passed
@blaisb blaisb deleted the flow_rates_postprocessing branch April 18, 2024 06:24
M-Badri pushed a commit to M-Badri/lethe that referenced this pull request Sep 29, 2024
Description of the problem
The previous flow rate post processing tool would add a line to the .dat file for each defined boundary (in the boundary condition subsection of the .prm file) for every time iteration. This is not the expected output as one would expect a single line to be added for each time step with the flow rates of every boundary.
Also, the values of the flow rates were added with a 12 digits precision. Instead, a 4 digits precision in scientific notation was chosen, which seems like a reasonable precision.
Description of the solution
The value of time is now added outside of the loop on the boundary conditions.
Comments
At the moment there is no unified output format : with/without scientific notation, different numbers of digits. This could be the work of a future PR.

Former-commit-id: 79f3d07
blaisb pushed a commit that referenced this pull request Sep 30, 2024
Description of the problem
The previous flow rate post processing tool would add a line to the .dat file for each defined boundary (in the boundary condition subsection of the .prm file) for every time iteration. This is not the expected output as one would expect a single line to be added for each time step with the flow rates of every boundary.
Also, the values of the flow rates were added with a 12 digits precision. Instead, a 4 digits precision in scientific notation was chosen, which seems like a reasonable precision.
Description of the solution
The value of time is now added outside of the loop on the boundary conditions.
Comments
At the moment there is no unified output format : with/without scientific notation, different numbers of digits. This could be the work of a future PR.

Former-commit-id: 79f3d07
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