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

AxialGeoMultiscale Tutorial #308

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

ezonta
Copy link

@ezonta ezonta commented Nov 16, 2022

The goal of this PR is to have a tutorial case for the newly developed AxialGeoMultiscale mapping feature.
The tutorial consists of a 1D and a 3D pipe that exchange velocity and pressure values.

This PR is still a draft, because the tutorial case is not yet satisfactory - TO DO:

  • Fix data not being exchanged and Fluid2 running independently
  • Change substitute 1D script into correct Nutils implementation
  • Clean up case and write README.md

@ezonta ezonta marked this pull request as ready for review November 24, 2022 13:30
@ezonta
Copy link
Author

ezonta commented Nov 29, 2022

There's still a bug in the tutorial regarding the run scripts. Running the tutorial from the participant folders with

cd Fluid3D && ./run.sh

does not work. However, executing both from the tutorial folder (using the runFluid3D script) works fine. I am not sure why that is.
With help from @MakisH, the cause could be found and the participants now run with the run.sh scripts found in their respective folders.

Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

This is already quite clean, thank you for the careful work! I added a few comments, mostly on details.

I have not yet tried to run it.

partitioned-pipe-multiscale/README.md Outdated Show resolved Hide resolved
partitioned-pipe-multiscale/README.md Outdated Show resolved Hide resolved
partitioned-pipe-multiscale/README.md Outdated Show resolved Hide resolved
partitioned-pipe-multiscale/README.md Outdated Show resolved Hide resolved
partitioned-pipe-multiscale/README.md Outdated Show resolved Hide resolved

simpleCoeffs
{
n (2 1 1);
Copy link
Member

Choose a reason for hiding this comment

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

With this, we are splitting per x-direction, meaning that only one rank will be participating in the coupling (assuming that the axis of the pipe is x).

It would be interesting (not necessarily in this PR) to check if the mapping also works in parallel when the interface is split between MPI ranks. I expect surprises.

@ezonta ezonta requested a review from MakisH January 24, 2023 09:07
Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

Some comments to get you started.

I want to look again into the Nutils code after you add some more comments, and I still need to run it. Since I assume you are still based on preCICE v2, please don't yet port it to v3.

</mesh>

<participant name="Fluid1D">
<use-mesh name="Fluid1D-Mesh" provide="yes" />
Copy link
Member

Choose a reason for hiding this comment

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

Note that we will soon need to convert this file (and some of the code) to preCICE v3: https://precice.org/couple-your-code-porting-v2-3.html

But let's only do it after merging the preCICE branch to develop, otherwise it will be a bit chaotic to test.

Comment on lines +23 to +31
readData
(
Velocity
);

writeData
(
Pressure
);
Copy link
Member

Choose a reason for hiding this comment

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

In his recent thesis, @thesamriel developed some new boundary conditions that would be interesting to try out here. No need to do it now, unless we really have issues on the interface. But you may want to have a look: https://mediatum.ub.tum.de/node?id=1696254&change_language=en

<m2n:sockets from="Fluid1D" to="Fluid3D" exchange-directory=".." />

<coupling-scheme:serial-implicit>
<time-window-size value="0.01" />
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need to couple so often? How important is this choice?
I assume that OpenFOAM still adapts its time step, but still, it currently looks like this is a requirement.


<coupling-scheme:serial-implicit>
<time-window-size value="0.01" />
<max-time value="20.0" />
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to run the case for so long to see clear results? For tutorials, I would prefer the case finishing in < 2 min runtime. I have not yet tried to run the case, though.

@@ -0,0 +1,5 @@
TimeWindow TotalIterations Iterations Convergence
Copy link
Member

Choose a reason for hiding this comment

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

This file should not be here.

partitioned-pipe-multiscale/README.md Show resolved Hide resolved
vertex_ids = interface.set_mesh_vertex(mesh_id, positions)
precice_dt = interface.initialize()

# problem definition
Copy link
Member

Choose a reason for hiding this comment

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

Could you write a few sentences on what exactly you are trying to solve here?
I have zero experience with nutils, so it could be useful if at least @IshaanDesai could also do a quick check. I think you have discussed it already.

ucons = diricons
stringintegral = '(p - ' + str(numpy.rint(p_read)) + ')^2 dS'
psqr = domain.boundary['right'].integral(stringintegral @ ns, degree=2)
pcons = solver.optimize('p', psqr, droptol=1e-15)
Copy link
Member

Choose a reason for hiding this comment

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

I barely understand what is going on here, but a tolerance of 1e-15 sounds too strict to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants