Add channel-transport transport OpenFOAM#551
Add channel-transport transport OpenFOAM#551fsimonis wants to merge 28 commits intoprecice:developfrom
Conversation
MakisH
left a comment
There was a problem hiding this comment.
Besides the fact that solver and config are in the same directory, it looks good overall. I have not yet tried to run it.
There was a problem hiding this comment.
I start feeling very uneasy that we duplicate this file everywhere.
Opened an issue: #559
There was a problem hiding this comment.
There is no uniform style in the tutorials repo. I don't see an easy solution for this.
There was a problem hiding this comment.
There is a .clang-format file in the root directory: https://github.com/precice/tutorials/blob/develop/.clang-format
Let's discuss in #559.
There was a problem hiding this comment.
We should have these in the central gitignore: https://github.com/precice/tutorials/blob/develop/.gitignore
The 0/ is already covered, the rest could go there as-is (strange we don't have them already).
There was a problem hiding this comment.
It's the other way around.
0/ is tracked and 0.*/ ignored by default. In my case I generate 0/ from 0.orig/, which is the exact opposite of what the gitignore defines.
Lines 41 to 43 in 3f253cc
There was a problem hiding this comment.
Interestingly, we don't have a .gitignore in https://github.com/precice/tutorials/tree/develop/partitioned-pipe-two-phase, which should have the same issue.
But again, the Make/-related rules can go into the central .gitignore, or at least moved to the solver directory.
There was a problem hiding this comment.
Can you explain the exact history of this file?
What did you get from where, what did you add yourself?
There was a problem hiding this comment.
Where would you expect to find this information? In the file itself or in a README in the directory of the solver?
There was a problem hiding this comment.
Just here first, to decide on what to do with this header.
But eventually in the file itself.
There was a problem hiding this comment.
A comment here would be nice.
I assume you are initializing a blob of T at (1,1).
| /*--------------------------------*- C++ -*----------------------------------*\ | ||
| | ========= | | | ||
| | \\ / F ield | OpenFOAM: The Open Source CFD Toolbox | | ||
| | \\ / O peration | Version: v2312 | | ||
| | \\ / A nd | Website: www.openfoam.com | | ||
| | \\/ M anipulation | | | ||
| \*---------------------------------------------------------------------------*/ |
There was a problem hiding this comment.
These headers are actually misleading, since we anyway have changed these configuration files a lot compared to what you might have started from.
(referring to this file and the fvSolution)
There was a problem hiding this comment.
How do you suggest me to handle this? (also in the solver source)
There was a problem hiding this comment.
I am suggesting to remove the copyright header from the fvSchemes and fvSolution, because it is wrong.
There was a problem hiding this comment.
What about the actual solver sources?
There was a problem hiding this comment.
There we need it, since it is actually a small extension of OpenFOAM code.
But this is discussed in #551 (comment)
There was a problem hiding this comment.
I replaced the header with a link and description. Does this clarify the matter?
|
@fsimonis this seems to be close to merging. Does this need more work from your side, or just a new review iteration? edit: Actually, no. There is still a blocking modeling issue. |
|
Just finished a co-working session with @fsimonis. I pushed some new commits, which make the custom solver work with dynamic meshes and adjusts the case to apply dynamic mesh refinement with The settings are in Run with: The next step would be to further understand and adjust the mesh refinement settings. The coupled case currently fails, but this is expected since the adapter does not yet support remeshing (precice/openfoam-adapter#382). I can start an implementation of that. The accumulation of divSchemes
{
default none;
- div(phi,T) Gauss linearUpwind grad(T);
+ div(phi,T) Gauss linear grad(T);
}However, the numerics and the boundary conditions are the same as in the pitzDaily case, so this might be unrelated. I would look next if the changes from the standard Some URLs for reference: |
|
I got the non-AMR version running. I moved the AMR specific changes to #689 and will revert this PR to the non-AMR version. |
b1f4d6a to
44e2384
Compare
|
@MakisH I think I implemented all suggestions. Could you review again? |
This PR adds an OpenFOAM variant of the transport participant of the channel-transport tutorial.
The case uses a modified version of the scalarTransportFoam application. Main difference is that we assume$\phi$ every timestep.
Uto change every timestep. Therefore we need to recompute@MakisH Can you give this PR a thorough review to make it as easy to use as possible?
Result:
Screencast_20251210_145936.webm
Checklist:
changelog-entries/<PRnumber>.md.edit: updated to the working state