Tracer flow rate at boundaries #1197

oguevremont · 2024-07-16T17:06:38Z

Description

An application of the tracer physics is to obtain the residence time distribution (RTD) of a flow through a domain. This PR adds the post-processing option that outputs the tracer flow rate at every boundary, providing the data for RTD analyses.

In addition, a bug where the conditions to allow boundary conditions to update their values were too strict was rectified. Each physics is now responsible for deciding when to update their boundary conditions, and this condition was already based on whether boundary conditions were time-dependent.

Testing

Documentation

Miscellaneous (will be removed when merged)

Checklist (will be removed when merged)

See this page for more information about the pull request process.

Code related list:

All in-code documentation related to this PR is up to date (Doxygen format)
Lethe documentation is up to date
New feature has unit test(s) (preferred) or application test(s), and restart files are in the generator folder
The branch is rebased onto master
Changelog (CHANGELOG.md) is up to date
Code is indented with indent-all and .prm files (examples and tests) with prm-indent

Pull request related list:

Labels are applied
There are at least 2 reviewers (or 1 if small feature) excluding the responsible for the merge
If this PR closes an issue or is related to a project, it is linked in the "Projects" or "Development" section
If the fix is temporary, an issue is opened
The PR description is cleaned and ready for merge

AmishgaAlphonius

Good work! I have a few comments :)

include/solvers/tracer.h

include/core/parameters.h

include/solvers/tracer.h

applications_tests/lethe-fluid/tracer_flow_rate.prm

source/solvers/tracer.cc

OresteMarquis

Really nice! Nothing much to say, only clarifications for my understanding.

OresteMarquis · 2024-08-05T14:11:03Z

source/fem-dem/cfd_dem_coupling.cc

@@ -1655,24 +1655,11 @@ CFDDEMSolver<dim>::solve()
  while (this->simulation_control->integrate())
    {
      this->simulation_control->print_progression(this->pcout);
-      bool refinement_step;
-      if (this->simulation_parameters.mesh_adaptation.refinement_at_frequency)


Just curious to know why is those if conditions are removed from the solvers and fem-dem?

It's because in the update_boundary_conditions functions, the only condition to respect was whether boundary conditions are time dependent. This is already very restrictive and not expensive to check, so to simplify the code I remove the refinement_step boolean here. It had no effect on any test. I feel like the previous condition was difficult to understand and didn't bring much.

If it does the same I like it! It is indeed much clearer :)

source/solvers/tracer.cc

hepap

Nice work! Sorry for the delay in my review!

I think your application test is enough, but a unit test could be also nice :)

source/core/parameters.cc

hepap · 2024-08-05T15:17:32Z

CHANGELOG.md

@@ -5,6 +5,12 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).

 ## [Master] - 2024-08-05

+### Added
+
+- MINOR A post-processing feature for the tracer flow rate through boundaries is added. This allows to produce data useful for residence time distribution analyses. [#1197](https://github.com/chaos-polymtl/lethe/pull/1197)


I think you should add the fix on the bc update

AmishgaAlphonius

one small correction left
Nice work! :)

include/solvers/tracer.h

blaisb

One small refactoring left and this should be good to go.

blaisb · 2024-08-05T20:42:45Z

source/solvers/tracer.cc

+  // Fill table
+  this->tracer_flow_rate_table.add_value(
+    "time", this->simulation_control->get_current_time());
+  this->tracer_flow_rate_table.set_precision("time", 12);
+  for (unsigned int i_bc = 0;
+       i_bc < this->simulation_parameters.boundary_conditions.size;
+       ++i_bc)
+    {
+      this->tracer_flow_rate_table.add_value("flow-rate-" +
+                                               Utilities::int_to_string(i_bc,
+                                                                        2),
+                                             tracer_flow_rate_vector[i_bc]);
+      this->tracer_flow_rate_table.set_scientific(
+        "flow-rate-" + Utilities::int_to_string(i_bc, 2), true);
+    }
+
+  // Console output
+  if (simulation_parameters.post_processing.verbosity ==
+      Parameters::Verbosity::verbose)
+    {
+      this->pcout << "Tracer flow rate at the boundaries: " << std::endl;
+      for (unsigned int i_bc = 0;
+           i_bc < this->simulation_parameters.boundary_conditions.size;
+           ++i_bc)
+        this->pcout << "\t boundary " << i_bc << ": "
+                    << tracer_flow_rate_vector[i_bc] << std::endl;
+    }
+
+  if ((simulation_control->get_step_number() %
+         this->simulation_parameters.post_processing.output_frequency ==
+       0) &&
+      Utilities::MPI::this_mpi_process(mpi_communicator) == 0)
+    {
+      std::string filename =
+        simulation_parameters.simulation_control.output_folder +
+        simulation_parameters.post_processing.tracer_flow_rate_output_name +
+        ".dat";
+      std::ofstream output(filename.c_str());
+      this->tracer_flow_rate_table.write_text(output);
+    }


Personally I would fill the table write the output outside of the function and not inside. The function calculates the flow rate, it does not really take care of anything else. This should be two functions. One two calculate and one to store the information.

I split into 2 functions as suggested. It makes more sense that way

blaisb · 2024-08-06T00:07:52Z

@oguevremont can you rebase and then I'll be able to merge

Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com>

Co-authored-by: hepap <47506601+hepap@users.noreply.github.com>

Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com>

Description An application of the tracer physics is to obtain the residence time distribution (RTD) of a flow through a domain. This PR adds the post-processing option that outputs the tracer flow rate at every boundary, providing the data for RTD analyses. In addition, a bug where the conditions to allow boundary conditions to update their values were too strict was rectified. Each physics is now responsible for deciding when to update their boundary conditions, and this condition was already based on whether boundary conditions were time-dependent. Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com> Co-authored-by: hepap <47506601+hepap@users.noreply.github.com> Former-commit-id: fd4481e

oguevremont force-pushed the tracer_postprocessing branch 2 times, most recently from d560eee to f7c5794 Compare July 16, 2024 17:35

oguevremont marked this pull request as ready for review July 16, 2024 17:39

oguevremont requested a review from AmishgaAlphonius July 16, 2024 17:39

oguevremont self-assigned this Jul 16, 2024

oguevremont added Enhancement New feature or request Ready for review Post-processing Modifications to the post-processing module labels Jul 16, 2024

AmishgaAlphonius reviewed Jul 16, 2024

View reviewed changes

oguevremont requested a review from hepap July 16, 2024 20:16

oguevremont requested a review from OresteMarquis August 5, 2024 13:27

OresteMarquis approved these changes Aug 5, 2024

View reviewed changes

oguevremont force-pushed the tracer_postprocessing branch from 4d3d401 to 4c9b8da Compare August 5, 2024 15:10

hepap approved these changes Aug 5, 2024

View reviewed changes

AmishgaAlphonius approved these changes Aug 5, 2024

View reviewed changes

include/solvers/tracer.h Outdated Show resolved Hide resolved

oguevremont requested a review from blaisb August 5, 2024 16:01

blaisb requested changes Aug 5, 2024

View reviewed changes

oguevremont force-pushed the tracer_postprocessing branch from 1fa519b to 9d076f6 Compare August 5, 2024 21:24

blaisb approved these changes Aug 6, 2024

View reviewed changes

oguevremont and others added 10 commits August 5, 2024 20:12

Add parameters for postprocessing tracer flow rates

2609109

Initialize tracer output parameters

b6f3264

Working V1

6ca00c1

WIP integrating diffusivity model

96961f4

Removed redundant conditions for updating BC

fbe286d

Integrate levelset dependence of diffusivity model

f6c3a59

Prepare for PR

21e36e4

Prepare for PR

4741bac

Apply suggestions from code review

c660fd5

Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com>

Address Amishga's review comments

eae42ab

oguevremont and others added 5 commits August 5, 2024 20:12

Update source/core/parameters.cc

a1a0f1c

Co-authored-by: hepap <47506601+hepap@users.noreply.github.com>

Updated changelog

20b9416

Update include/solvers/tracer.h

46b7314

Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com>

Refactor calculations and output

9a43b72

Fixed changelog

28693db

oguevremont force-pushed the tracer_postprocessing branch from 9d076f6 to 28693db Compare August 6, 2024 00:14

blaisb merged commit fd4481e into master Aug 6, 2024
8 checks passed

blaisb deleted the tracer_postprocessing branch August 6, 2024 01:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tracer flow rate at boundaries #1197

Tracer flow rate at boundaries #1197

oguevremont commented Jul 16, 2024 •

edited by OresteMarquis

Loading

AmishgaAlphonius left a comment

OresteMarquis left a comment

OresteMarquis Aug 5, 2024

oguevremont Aug 5, 2024

OresteMarquis Aug 5, 2024

hepap left a comment

hepap Aug 5, 2024

AmishgaAlphonius left a comment

blaisb left a comment

blaisb Aug 5, 2024

oguevremont Aug 5, 2024

blaisb commented Aug 6, 2024

Tracer flow rate at boundaries #1197

Tracer flow rate at boundaries #1197

Conversation

oguevremont commented Jul 16, 2024 • edited by OresteMarquis Loading

Description

Testing

Documentation

Miscellaneous (will be removed when merged)

Checklist (will be removed when merged)

AmishgaAlphonius left a comment

Choose a reason for hiding this comment

OresteMarquis left a comment

Choose a reason for hiding this comment

OresteMarquis Aug 5, 2024

Choose a reason for hiding this comment

oguevremont Aug 5, 2024

Choose a reason for hiding this comment

OresteMarquis Aug 5, 2024

Choose a reason for hiding this comment

hepap left a comment

Choose a reason for hiding this comment

hepap Aug 5, 2024

Choose a reason for hiding this comment

AmishgaAlphonius left a comment

Choose a reason for hiding this comment

blaisb left a comment

Choose a reason for hiding this comment

blaisb Aug 5, 2024

Choose a reason for hiding this comment

oguevremont Aug 5, 2024

Choose a reason for hiding this comment

blaisb commented Aug 6, 2024

oguevremont commented Jul 16, 2024 •

edited by OresteMarquis

Loading