Fix HF Sign Direction + Patch Non-Restart Bug in SU2
- Heat flux written to preCICE was of the wrong sign for CHT -- this PR fixes that
- Patches #42
- ~Cleans up whitespaces (all converted to tabs)~
Sorry for taking so long to address the comments here! I have been super busy.
I force pushed commit updates (hopefully that was OK here) to deal with the whitespace inconsistencies between commits. Please let me know if there are any other comments.
Thank you very much for the continuous contributions, @j-signorelli! I totally understand the long intervals, happens to all of us all the time.
Could you please edit the first post to describe a bit more verbose on what issue this PR solves, and how to test the issue and the fix?
@MakisH Done. Again apologies for taking so long to get back to you.
Let me know any thoughts, particularly on the 2nd issue this resolves. Alternatively it may be better to just explain this in more detail and advise for using restart files (and update the flow-over-heated-plate example to use one), instead of trying to patch an inconsistency here.
Issue 1: Heat flux direction
If I understand correctly, the heat flux assumption is now the same as in OpenFOAM:
https://github.com/precice/openfoam-adapter/blob/cf65114a25b17ceb3daaec82107fdc53043e6b71/CHT/HeatFlux.C#L21-L57
But why was it working before? What exactly does the -r option do?
With the inclusion of a flip-normal flag indicated by https://github.com/precice/preeco-orga/issues/42
This sentence seems incomplete.
Issue 2: Results time
Do we now have different behavior in cases that use restart files and cases that do not? This is definitely something we should document.
If we are to patch it, then we need to remember to change it back to the state that it was before the fix in SU2. But I assume that any fix would only be released for the latest SU2 version anyway.
My gut feeling tells me that we should address this with documentation, not with an always-on patch on our side that might have side-effects in other cases. But happy to get convinced otherwise.
-
I updated the description - I had meant without the
-rflag. It doesn't appear to be an issue before because the tutorial case (flow-over-heated-plate) uses the-rflag. It has been shown that Fluid-Dirichlet/Solid-Neumann is favorable over Fluid-Neumann/Solid-Dirichlet (see here - as such, this is the default setting. The-rflag reverses this coupling, as is done for flow-over-heated-plate. -
Yes exactly. The tutorial for flow-over-heated-plate will not be correct without either A. this patch or B. the inclusion of a restart file as the IC. I also think that documentation may be better. I can remove that work from this PR, and ensure both updated documentation and a PR with an IC restart file for flow-over-heated-plate - just give me confirmation!
-
I found a glaring issue that I seem to have missed regarding data initialization for CHT and pushed a commit resolving that as well.
- I updated the description - I had meant without the
-rflag. It doesn't appear to be an issue before because the tutorial case (flow-over-heated-plate) uses the-rflag. It has been shown that Fluid-Dirichlet/Solid-Neumann is favorable over Fluid-Neumann/Solid-Dirichlet (see here - as such, this is the default setting. The-rflag reverses this coupling, as is done for flow-over-heated-plate.
Another good reason to have configurable normal vector flipping. Good to have the -r flag, later we could add this to the adapter config.
I can remove that work from this PR, and ensure both updated documentation and a PR with an IC restart file for flow-over-heated-plate - just give me confirmation!
Yes, please, that would be great. In any case, in the future, one topic per PR would make reviewing and merging easier.
Yeah agreed this was a bit confusing. I will keep this PR for fixing heat flux sign and initialization of CHT data. I'll submit a separate PR for adding documentation on restart files, and a PR to the tutorials repo with an IC.
This is now done and ready for review.
No updates needed in regards to this PR -- however, #47 requires an update restarting with an initial condition to the tutorial case. I'll get that going (I think I have one already).