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

Integrated Heatflux for flow problems #1530

Merged
merged 16 commits into from
Feb 2, 2022
Merged

Conversation

TobiKattmann
Copy link
Contributor

@TobiKattmann TobiKattmann commented Jan 31, 2022

Hi,

Possibility to give the Heatflux over a boundary in [W] instead of [W/m^2]. This was already possible in the HeatSolver but now also in Inc and Comp flow solvers.
Prescribing a fixed Watt is especially useful in optimization scenarios with Temperature OF where otherwise the optimizer can exploit that a smaller boundary almost certainly yields smaller Temperatures.

The necessary SurfaceAreas are computed and stored in CGeometry instances which is computed once at program start by a call from the Driver.

I also added NEMO for this feature.

There is currently no Testcase which uses INTEGRATED_HEATFLUX= YES so one might want to change that 😬 🤫

For the HeatSolver this was originally introduced at some point by @oleburghardt

PR Checklist

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags, or simply --warnlevel=2 when using meson).
  • My contribution is commented and consistent with SU2 style.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp) , if necessary.

Therefore a SurfaceArea routine was added to CGeometry which is called once
in the GeoPreproc of the Driver. This central Surface area comp makes the
HF change straight forward and might be useful in some other cases.
The ordering of the SurfaceArea vector of CGeometry follows the Global cfg ordering.
Common/include/geometry/CGeometry.hpp Outdated Show resolved Hide resolved
* \brief Compute the surface area of all global markers.
* \param[in] config - Definition of the particular problem.
*/
void ComputeSurfaceArea(const CConfig *config);
Copy link
Member

Choose a reason for hiding this comment

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

Call it in CGeometry::UpdateGeometry or the sensitivities will be incorrect.
Might also be needed for the OF in multizone adjoints?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added the call in updateGeometry in 288a2eb . UpdateGeometry is only ever called for deforming meshes. When I moved the ComputeSurfaceArea routine only to UpdateGeometry my case segfaulted because ComputeSurfaceArea was never called and the vector never resized and filled. The call to ComputeSurfaceArea coming from the CDriver ctor (via Geometrical_Preprocessing) is always evaluated at startup (except for DG-FEM cases)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am unsure where you think this would go wrong for multizone adjoints 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Yes I said to call it, not to move it 😉
Just thinking of the places where Set_Heatflux_Areas was used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to tackle Set_Heatflux_Areas at another point anyway as it is somewhat connected to Heat_Fluxes which computes OF's and hist-output for the HeatSolver. And in there I dont like 2 things:

  1. On isothermal walls, the cfg T is used for HF computation. The Isothermal wall is weakly enforced so this could create quite a different value compared to a purely postprocessed (eg via paraview) result
  2. AvgTemp is only reported for Heatflux-walls (not even for CHT-interfaces). Imo it should be users choice where he wants which quantities even if they are trivial

I reverted the cut-down of Set_Heatflux_Areas here e198acf because I would need to care about above points and that might be better done in a separate PR

SU2_CFD/src/solvers/CIncNSSolver.cpp Outdated Show resolved Hide resolved
Common/src/geometry/CGeometry.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CNSSolver.cpp Outdated Show resolved Hide resolved
makes comparing cases a little less annoying
Used a 3D cht case and applied it in the solid zone. Residual stay exactly the same.
This, to my experience is only the case because the surface is a plane rectangle.
For e.g. a round shape the small mismatch in exact circle circumference and the discrete
appriximation by the mesh is enough to alter the residuals (and final temperature)
slighthly. The Residual change in this case for the Temperature is due to the error that
was in the mesh (see Testcases #91)
@TobiKattmann
Copy link
Contributor Author

TobiKattmann commented Jan 31, 2022

Thanks already @pcarruscag for the quick suggestions 👍 Let me know if there are more :)

2 linked PR's go with this:

  1. The testases PR needs to be merged before this to make sure the reg tests works (Edit: Done and reg tests work (except hybrid_par_AD...))
  2. The website update (...has more time ;)

Otherwise I am happy so far with this PR

@TobiKattmann TobiKattmann mentioned this pull request Feb 2, 2022
5 tasks
@pr-triage pr-triage bot removed the PR: unreviewed label Feb 2, 2022
@TobiKattmann
Copy link
Contributor Author

Thanks for the reviewing @pcarruscag 💐 I am merging this in then. And then afterwards the website PR su2code/su2code.github.io#69

@TobiKattmann TobiKattmann merged commit 1502831 into develop Feb 2, 2022
@TobiKattmann TobiKattmann deleted the feature_integratedHF branch February 2, 2022 15:22
TobiKattmann added a commit that referenced this pull request Feb 2, 2022
This is due to the previously merged PR #1530 where an NDIME error in the mesh was fixed (where the reg test values already changed) and as this PR makes the same case a restarted one... the values have to be adapted again
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