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

Fix HeatFlux non-dimensionalization #2109

Closed
wants to merge 3 commits into from
Closed

Conversation

TobiKattmann
Copy link
Contributor

Proposed Changes

Hi all,

volume heatflux output is not non-dimensional which is inconsistent with how the rest of volume output is handled.
Note that the History outputs (like TOTAL_HEATFLUX over a bunch of surfaces) is re-dimensionalized -> Apparently that hasn't bothered too many people until now but generally that might be sth we/I want to change (?) Let me know

I fixed it in a way that follows the current convention.

In CConjugateHeatInteface.cpp the required heat_flux is (re)computed on its own so I do not expect a problem there but I will check manually as well

  • Reminder Checkmark to actually do so
    If there is other places which might be affected let me know.

PR Checklist

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • 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.

TobiKattmann added 2 commits August 14, 2023 15:11
…alization.

We re-dimensionalize aggregated surface outputs for SCREEN_OUTPUT, but we do not for volume output.
So by appliying the re-dim one step later we keep the nondim HeatFlux container which is used for volume output as well.
@j-signorelli
Copy link
Contributor

I also found out the hard way that TOTAL_HEATFLUX was dimensional, but one additional thing to consider here is that if I call CDriverBase::GetMarkerNormalHeatFlux in a Python-wrapped code then I think this PR would cause the returned heat flux to be nondimensional, which can be a nuisance if using it for CHT while running in nondimensional mode. If for FSI, CDriverBase::GetMarkerFlowLoad is the dimensional flow load, then this PR would also be inconsistent with that; but if that function returns nondim. loads then this update makes much more sense (I am not sure if it's nondim or not).

@TobiKattmann
Copy link
Contributor Author

TobiKattmann commented Aug 15, 2023

@j-signorelli afai see this PR would indeed cause CDriverBase::GetMarkerNormalHeatFlux to return nondimensional HeatFlux (compared to dimensional HeatFlux in current master) and could trouble people there ... but in doubt we can still dimensionalize in that function to keep the python wrapper as-is. (But there might be a cleaner solution to this)

As I do not know much about the structural solver in SU2, I cannot say with confidence -> but from what I saw from skimming over CFEA(Base)Solver, config_template, and a handful of Elasticity cfg's there is no nondimensionalization for the structural solver (but somebody with better knowledge of that should educate me here).

Edit: The HeatSolver's HeatFlux written dimensional as well -> so I guess that should be changed as well ... or at least the whole thing should be consistent across solvers

@pcarruscag
Copy link
Member

Let's keep the dimensional outputs, we are already trying to have dimensional inputs to keep things simple.
In the longer term we should aim to have dimensional inputs and outputs and make the non-dimensionalization an internal detail of the solvers that users don't have to worry with.

@pcarruscag
Copy link
Member

Including volume outputs, which right now are a pain for restarting with different types of non-dim

@jaspe55
Copy link
Contributor

jaspe55 commented Sep 28, 2023 via email

@TobiKattmann
Copy link
Contributor Author

Hi jaspe55, I would have to check -> a possible explanation I can think of rn (without having checkde) is that we compute the heatflux on the wall based on the temperature gradient of wall temperature with an interior Temperature and that can lead to a non-zero heatflux while there is non at the boundary. Of course that should tend to zero upon refinement and for an adiabatic case.

Maybe I am on the wrong track here but I'll try to check soon

@pcarruscag
Copy link
Member

you are correct, there is a discussion on CFD-online about it. we impose 0 heat flux, but report an "apparent heat flux" most codes will simply give you back the imposed heat flux value you specify, nevertheless there will probably be a temperature gradient close to the wall

@jaspe55
Copy link
Contributor

jaspe55 commented Sep 29, 2023 via email

@jaspe55
Copy link
Contributor

jaspe55 commented Oct 23, 2023 via email

@pcarruscag
Copy link
Member

Is there a simple example to reproduce the issue?

@jaspe55
Copy link
Contributor

jaspe55 commented Oct 23, 2023 via email

@jaspe55
Copy link
Contributor

jaspe55 commented Oct 24, 2023 via email

@pcarruscag
Copy link
Member

Yes that makes sense, we are probably missing some validation on markers not appearing on incompatible boundary conditions.

@jaspe55
Copy link
Contributor

jaspe55 commented Oct 25, 2023 via email

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.

5 participants