MiguelMValero / CONES

CONES: This git repository aims to couple the CFD software OpenFOAM with any other kind of open-source codes. It is currently employed to carry out sequential Data Assimilation techniques and, more specifically, an Ensemble Kalman Filter (EnKF). The communications between the EnKF and OpenFOAM are performed by a coupler called CWIPI.
6 stars 1 forks source link

Porting to openfoam9 #5

Closed PaoloErrante closed 3 months ago

PaoloErrante commented 1 year ago

Hello everyone, I am trying to get CONES to work on openfoam9 and (next step will be openfoam10). As I try to compile (wmake) I stumble on this error:

cwipiPstream.C:774:60: error: use of deleted function ‘Foam::fvPatchField::fvPatchField(const Foam::fvPatchField&) [with Type = Foam::Vector]’ 774 | fvPatchVectorField movingWallU = vf.boundaryField()[top];

in Version 9 this is caused by: ``//- Disallow copy without setting internal field reference 197| fvPatchField(const fvPatchField&) = delete;

In Version 8 it works because this "constraint" is not given: 197| fvPatchField(const fvPatchField&)

Do you have some insights about a possible workaround?

villanul commented 1 year ago

Hello,

It seems to say that it needs a reference of the internal field in order to copy the field of a boundary in OF9, but the field is already a reference (?). Maybe it demands to copy the VolVectorField before copying its boundary.

It think you can By-pass the copy of the boundary field and directly call : ParamsToSend[0] = vf.boundaryField()[top][0].component(0);

It seems to be working on OF8 but i didn't verify the data passed to the EnKF.

Hope it helps !

PaoloErrante commented 1 year ago

Hello to everyone,

I am just opening this new branch to start to keep track of modification required for a porting to OpenFoam9. I think that there are still few things to do.

Here's a list of things done:

  1. Removed local paths for compilations and added local cwipi compiled library (No more need to have the user to compile cwipi0.12.0, this can be handy when there will be another cwipi version).
  2. Updated the path of the OpenFOAM libraries called (this might be critical, I am not sure how useful can be for cases other than the cavity_test)
  3. Fixed some warnings during compilation.

Not yet ready to be merged with master, there is still an MPI error at the end of the test-case when the solution has to be written.

Of course every suggestion is welcomed.

MiguelMValero commented 1 year ago

Hello Paolo,

Thank you so much for the post and your work. Do you think the porting to OpenFOAM 9 will be ready by the 11th April? I have just seen that TGCC is going to migrate from Red-Hat 7 to Red-Hat 8, and there is no OpenFOAM 8. So Lucas and I would need to use OpenFOAM 9 or 10 to be able to run cases on the cluster.

I can also help you. I will download OpenFOAM 9, and I can pull your branch to see if there is an easy way to deal with the MPI error at the end.

I will also try to upload the pitzDaily and plane Channel cases by the end of next week, to have two more examples with SimpleFoam and PimpleFoam, respectively (and let's hope in these two cases differences between OpenFOAM 9 and 9 are not too huge!). Fingers crossed...

PaoloErrante commented 1 year ago

Hi Miguel,

Thank you for your support.

To answer your question; that depends on how fast we can solve this MPI error and run all the testcases.

But before: I will have to make adjustments coherently to the merge of your branch and Lucas one. I will update the branch as soon as it compiles again.

Then we can check together how to solve this MPI issue.

PaoloErrante commented 1 year ago

Here's a valgrind summary for the memory leakages

LEAK SUMMARY: definitely lost: 19,910 bytes in 40 blocks indirectly lost: 23 bytes in 2 blocks possibly lost: 0 bytes in 0 blocks still reachable: 26,042 bytes in 205 blocks of which reachable via heuristic: newarray : 4,736 bytes in 28 blocks suppressed: 0 bytes in 0 blocks ERROR SUMMARY: 210 errors from 47 contexts (suppressed: 0 from 0)

villanul commented 1 year ago

Hello Paolo and Miguel,

Thank you for you work.

Does valgrind indicates the variables responsible for the memory leakage ?

Do you guys plan to keep two branches for OF8 and OF9 ? Or only keep the OF9 one ?

PaoloErrante commented 1 year ago

Hello Lucas,

yes Valgrind allows to find easily which elements of the code are involved in the memory leakage. Unfortunately for us they are 210. Here's an example:

==243678== Invalid write of size 2 <- This indicates that a value of 2 bytes is written not in a correct way ==243678== at 0x48529E3: memmove (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==243678== by 0x7EEE6C3: opal_convertor_unpack (in /usr/lib/x86_64-linux-gnu/libopen-pal.so.40.30.2) ==243678== by 0xCD46DD8: mca_pml_ob1_recv_request_progress_match (in /usr/lib/x86_64-linux-gnu/openmpi/lib/openmpi3/mca_pml_ob1.so) ==243678== by 0xCD4ADA9: mca_pml_ob1_recv_req_start (in /usr/lib/x86_64-linux-gnu/openmpi/lib/openmpi3/mca_pml_ob1.so) ==243678== by 0xCD4C753: mca_pml_ob1_recv (in /usr/lib/x86_64-linux-gnu/openmpi/lib/openmpi3/mca_pml_ob1.so) ==243678== by 0x4909464: PMPI_Recv (in /usr/lib/x8664-linux-gnu/libmpi.so.40.30.2) ==243678== by 0x6E87161: Foam::cwipiRecvParams(Foam::fvMesh const&, Foam::GeometricField<Foam::Vector, Foam::fvPatchField, Foam::volMesh>&, int, int, int, float) (in /home/paolo/OpenFOAM/paolo-9/platforms/linux64GccDPInt32Opt/lib/libcwipiPstream.so) **<- This indicates that it has something to do with cwipiRecvParams ==243678== by 0x13648C: main (in /home/paolo/OpenFOAM/paolo-9/platforms/linux64GccDPInt32Opt/bin/cwipiIcoFoam) <- Which has been called by cwipiIcoFoam _ ==243678== Address 0x8ce8d20 is 0 bytes after a block of size 0 alloc'd ==243678== at 0x484A2F3: operator new[](unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) Here is telling us that is caused by an operator to initialize an array ** ==243678== by 0x6E87119: Foam::cwipiRecvParams(Foam::fvMesh const&, Foam::GeometricField<Foam::Vector, Foam::fvPatchField, Foam::volMesh>&, int, int, int, float) (in /home/paolo/OpenFOAM/paolo-9/platforms/linux64GccDPInt32Opt/lib/libcwipiPstream.so) ==243678== by 0x13648C: main (in /home/paolo/OpenFOAM/paolo-9/platforms/linux64GccDPInt32Opt/bin/cwipiIcoFoam)

MiguelMValero commented 1 year ago

Thank you, Paolo, for your work! I think we'll also take a look at this on Friday...

About your question, Lucas, I'd keep the code in both versions of OpenFOAM. Maybe for the master branch, we can leave the version of OpenFOAM-v9, but I'd include an additional branch with the case in OpenFOAM-v8.

MiguelMValero commented 1 year ago

I've just checked and Pstream library in versions 8 and 9 are identical. There is only a small difference in two comments of the file PstreamGlobal.H. So the error in OpenFOAM9 is not coming from there! image

PaoloErrante commented 1 year ago

That's true @MiguelMValero. So have you tried to compile and run CONES with 9? Do you get any error? If yes, is it the same error I get?

MiguelMValero commented 1 year ago

Hello, yes, I compiled it in OpenFOAM9, I substituted the compilation errors and I ran it in the same way as you did. I got exactly the same error. I am not very convinced about the new declaration

fvPatchVectorField movingWallU = vf.boundaryField()[top]; in OpenFOAMv8 double movingWallU = vf.boundaryField()[top][0].component(0); in OpenFOAMv9

And I also want to check the change of OpenFOAM libraries that you have done. I think we are getting closer.

MiguelMValero commented 1 year ago

Hello,

I tried both the "cavity test" and the "pitzDaily case", and they seem to be working on OpenFOAM9, without the MPI error at the end. Hence, I uploaded my branches to the OpenFOAM9 branch. Hope it works for you as well!

PaoloErrante commented 1 year ago

Just pulled your updates Miguel, everything works fine for me, without any MPI errors! Just a small thing: the cwipiSimpleFoamkOmegaSST does not compile. In the Make/options file there is still something taken from the v8 I think, if I link to the cwipiPstream in lib there is the fvOptions.H missing, as @villanul was telling us today. Probably we should include that header from v8 in the lib as I think Lucas did with his previous commit: lib/momentumTransportModels

That should be the definitive fix.

MiguelMValero commented 1 year ago

Hello,

About cwipiSimpleFoamKOmegaSST, it's normal. I didn't convert it from version 8 to 9. And also, the momentumTransport file inside the "constant" folder of the pitzDaily case must be modified for the free coefficients of the model. In fact, I didn't want to include it. My fault! And about the libraries, I will take a look at them over the week to check what it's missing.

Regards,

villanul commented 1 year ago

Hello everybody,

I updated the OF9 repository with the pimpleFoam solver and channel test.

I also corrected some things on other solvers regarding the changes between OF8 and OF9 on the fvoptions and also on the way OF manage the reference cell for pressure. All the solvers normally compile now.

I don't have any errors on the cavity and pitzdaily but still have the corrupted size bins error for the channel.

Hope i didn't overwrite anything this time.

PS : I also deleted all the files related to the compilation and calculation because think it shouldn't be on the repository

Regards,