precice / fenicsx-adapter

Experimental preCICE-adapter for the open source computing platform FEniCSx
GNU Lesser General Public License v3.0
10 stars 4 forks source link

Adapter precice 3.1 with 3D support and "point load" #26

Open efirvida opened 1 month ago

efirvida commented 1 month ago

Sorry If I dont follow the source style here, my point was to quick develop a solution when it works, we can do it well. This also can keep the code more clean, to discuss over it. Also I have to say that I'm learing preCICE and FenicsX at the same time, so maybe the solution here will be simple and I can't notice it.

I post a first version of the adapter on the preCICE Disource because at that point I have where I call a scaling problem in the flap tutorial, having similar results but with 1 to100 diference in the result.

Then I create a simple static beam problem comparing the results of CaluliX and FenicsX and here I notice the problem, I was integrating the load over the surface, instead of on a DoF as Calculix does, which was confirmed by BenjaminRodenberg in the discourse. This is the example that I put on the test folder. So I update my the flap tutorial based on the lessons learned here.

This also guided by this other thread on FeniscX forum, about mapping the loads over DoFs. But I still having problems validting the flap tutorial. Considering that the FenicsX dos'nt have a PointSource as the legacy version, I use another approach of modifiying directly the vector value as:

LOAD_POINT = (4.9490529e-02, -1.4846031e-01, 1.5000000e02)
LOAD = np.array([10.0, 0, 0])

def point(x):
    return (
        np.isclose(x[0], LOAD_POINT[0])
        & np.isclose(x[1], LOAD_POINT[1])
        & np.isclose(x[2], LOAD_POINT[2])
    )

# https://bleyerj.github.io/comet-fenicsx/tours/dynamics/elastodynamics_newmark/elastodynamics_newmark.html
point_dof = fem.locate_dofs_geometrical(V, point)[0]
point_dofs = np.arange(point_dof * dim, (point_dof + 1) * dim)

b_func = fem.Function(V)
b_func.x.array[:] = 0
b = b_func.vector
with b.localForm() as b_local:
    b_local.set(0)

for i, j in enumerate(point_dofs.tolist()):
    b[j] = LOAD[i]
b.assemble()

Which works for a single point, but I'm missing somthing here, and I dont know if is on the preCICE side or in the implementation of the flap tutorial. Rigth now I can confirm that the adapter can handle 2D and 3D problems, but without good results.

  1. https://fenicsproject.discourse.group/t/improve-mapping-data-on-a-function-by-coordinates/14911/9
  2. https://precice.discourse.group/t/fsi-with-fenicsx-and-precice-3/1966
precice-bot commented 1 month ago

This pull request has been mentioned on preCICE Forum on Discourse. There might be relevant details there:

https://precice.discourse.group/t/fsi-with-fenicsx-and-precice-3/1966/3

efirvida commented 1 month ago

I finally made it work, at least the perpendicular flap tutorial, To do it I added a LinearDiscreteProblem that puts the load directly on the DoFs and also maintains the code clean.

flap-x-displacement-tip

I added two more examples the turek-heron, and a turbine-tower, right now I'm running them to see what happens. I'm having problems by running the turbine-tower in parallel from the OpenFOAM side, but I don't know how to diagnose what happen

precice-bot commented 1 month ago

This pull request has been mentioned on preCICE Forum on Discourse. There might be relevant details there:

https://precice.discourse.group/t/openfoam-crash-running-in-parallel-with-my-custon-dolfinx-adapter/2043/1

efirvida commented 1 month ago

@IshaanDesai thanks for your quick review. Yes, I use the original adapter structure, because first I tried to use the original on my FSI by doing the small modifications. As I'm new on preCICE and DOLFINx, I began to modify the code in place to do some reverse engineering, until I reached good results, and I removed a lot of code that I didn't understand to keep my version as clean as possible with the minimum needs to perform an FSI, as you can see the API on this version is a lot simpler than the original.

I know that I put a lot of tutorials that do not belong here, as de solver.py, but I think it is easier to have all things together in one place to test/review before moving/merge and put all in the right place. But OK I did some cleaning

The main part of the Adapter are the methods:

interpolation_points_in_vector_space
interface_dof

the first return the DOFs and coordinates of the interface. and then is easy to use the interface_dof to read and write data from the evaluated Function. I tried to add another tutorial on heat transfer but I've not time to do it to see if this approach also works for this kind of solution. Also, the Turek-FSI isn't working well I think because this is a nonlinear problem and I use a linear solver.

precice-bot commented 1 month ago

This pull request has been mentioned on preCICE Forum on Discourse. There might be relevant details there:

https://precice.discourse.group/t/how-mesh-coupling-works-for-parallel-participants/2059/1

IshaanDesai commented 1 month ago

Yes, I use the original adapter structure, because first I tried to use the original on my FSI by doing the small modifications. As I'm new on preCICE and DOLFINx, I began to modify the code in place to do some reverse engineering, until I reached good results, and I removed a lot of code that I didn't understand to keep my version as clean as possible with the minimum needs to perform an FSI, as you can see the API on this version is a lot simpler than the original.

I understand how your reached the state you have, and that it serves the purpose of doing FSI simulations. But the original structure is intended to be for generic use and was also designed rigorously for legacy FEniCS. I am tending towards keeping the old structure and accommodating your improvements in it. Did you encounter any fundamental design problems while trying to incorporate your implementation in the current adapter? If yes, we can improve the design.

I know that I put a lot of tutorials that do not belong here, as de solver.py, but I think it is easier to have all things together in one place to test/review before moving/merge and put all in the right place. But OK I did some cleaning

This is perfectly fine for now, but just something to do at the end, just before merging.

I tried to add another tutorial on heat transfer but I've not time to do it to see if this approach also works for this kind of solution.

This would be very interesting to know. If this can be done, we could potentially just move to the new design.

Also, the Turek-FSI isn't working well I think because this is a nonlinear problem and I use a linear solver.

This is a known problem.

I would suggest the following next steps:

  1. I can help you to try to incorporate your implementation into the current adapter design. This way we can keep the design but you can also do FSI simulations.
  2. Move your simulation setups to the tutorials repository one by one.
  3. If possible, add some tests for the new functionality.

What do you think? I will, of course, assist to a large degree here.

efirvida commented 1 month ago

I can try to adapt my implementation into the current adapter, but for this, I would need your help, I don't remember the problems in the main implementation.

I think that the main cut-off is on the initialize method where I believe the write_object is not needed here, or at least I didn't understand its function. Maybe due to the simple implementation of the read/write data methods, that I put the data directly on the Function.

And the same for the create/update_coupling_expression that I think we don't need here, at least for FSI.

Right now this version has a problem working in parallel, and I'm working on it right now, the I could try to start mergin with the current implementation. Notice that the current implementation also is for preCICE < 3, and this implementation not, so here some method changes its names and parameters.

IshaanDesai commented 1 month ago

I can try to adapt my implementation into the current adapter, but for this, I would need your help, I don't remember the problems in the main implementation.

I can and will definitely help here.

I see that you extract the data from the function directly at the time of reading and writing. I will check how this would work with the original implementation.

Handling FEniCS Expressions was part of the original design because many times Expressions were used on the user-side. We can rethink this now.

Right now this version has a problem working in parallel, and I'm working on it right now, the I could try to start mergin with the current implementation.

I saw the Discourse post about the issue with parallel runs. Let me try to answer there, so as to not clutter this pull request too much. Thanks for agreeing to merging with the current implementation :smile: I know it feels like taking a step backwards, but in the long run we would benefit from the original design decisions.

Notice that the current implementation also is for preCICE < 3, and this implementation not, so here some method changes its names and parameters.

Thanks for pointing this out :+1:

efirvida commented 1 month ago

I accidentally overwrote my last commit, but I believe it's not a significant issue. I modified the adapter using the current class reference to simplify the merging process.

In the current implementation, the read and write methods are much simpler because preCICE v3 does not differentiate between vectors and scalars, using the same function for both. I also removed some checks that I didn't fully understand, and I'm not certain if they were necessary.

        assert (self._coupling_type is CouplingMode.UNI_DIRECTIONAL_READ_COUPLING or
                CouplingMode.BI_DIRECTIONAL_COUPLING) 

As I mention in my last post, I'm porting the adapter to preCICEv3 so some methods change, now we have:

I also added some new properties:

So the API change a little bit due to this new properties, and the change of the preCICE version.

Also in the initialize method I left the write_object param just to maintain compatibility, but my current implementation don't need it. And also I remove all the check on this methods because I didn't understand, and I'm not sure if they were necessary.

FinallyI include the flap tutorial again with this modification, jus to show how we can use it.

efirvida commented 1 month ago

Also I manage to add a working heat transfer tutorial, in this commit of my main branch (which is not aligned right now with this last version). The tutorial runs until the end and change data between participants, but as I'm not an expert in heat transfer I'm sure that there are things wrong there on the FEniCSx side implementation, but it's a starting point