-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
Update the adapter for preCICE v3 and add test cases for Elmer-OpenFOAM coupling #4
Conversation
…mer to OpenFoam. A hard-coded naming convection was added to CoupleSolver.F90 for Elmer <-> OF coupling.
Ported the adapter for elmer from v2 to v3. Added a case to couple El…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot already! As this change is a bit larger, please be patient with the review 😁
Two things already to simplify the review. Please
- Remove the
precice-profiling
folders - Strip the
tools
to what you actually need
I have removed the precice-profiling and the extra files in the tools folder. Let me know if you need anything else. I am happy to help make this a great tool. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This log file can be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tapegoji thanks a lot for this contribution! Great work already 👏 Please have a look at my review comments. I did not run the test case and also did not have a look at the OpenFOAM things. After we do one iteration of the review, I can try running things. We can then finalize this pull request.
@tapegoji please let me know when the pull request is ready to be reviewed again. No rush, but I will wait for your ping to have a look again. |
@IshaanDesai, I am a bit confused what I need to do. can you explain what I should do. |
I have applied all the changes. let me know if I am missing anything. |
Any update on this? |
I'll review this week 👍 apologies for the delay, some other urgent things took time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are close to merging this. I have added some last comments. For the flow over heated plate tutorial, I see that you have copied over the OpenFOAM files from the precice/tutorials repository. Instead of copying the files here, a better idea would be to write a small README file here which describes how to copy Elmer files to the tutorials repository and run the tutorial there. This would avoid duplication. If it is not clear how to do this, I or someone else can do this.
Thanks once more for your contribution! Once you address the comments above, we can finalize this.
@tapegoji thanks for making the final changes. Adapter-wise we are ready to merge this. The next step would be to move the files relevant to the flow-over-heated-plate tutorial to the precice/tutorials repository. I can do this in the next days, and then we can test if everything works. Once we are sure, we can merge this pull request, and also merge Elmer as a new participant to the tutorial. |
The Elmer fluid and solid participants for the flow-over-heated-plate tutorial are being added to the appropriate folder in the precice/tutorials repository via: precice/tutorials#565 The cases, and also the OpenFOAM cases copied over for testing have now been deleted. The reference problem has also been removed. The I will continue cleaning up this pull request and finalizing the one mentioned above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are somethings I noticed while moving the case files to the precice/tutorials
repository.
Flow_Over_Heated_Plate3D/Reference_Problem/Reference_Problem_Mesh.msh
Outdated
Show resolved
Hide resolved
I am good with all the changes. Please go ahead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good to me
@tapegoji I opened a pull request in the tutorials repository to move the flow over heated plate case files to the respective folder: precice/tutorials#565 I tried running the case there, with both |
did you use the mesh from OpenFoam or did you use the mesh that I provided? |
I used the mesh which was in the folder of the case in this pull request. Let us continue the discussion in precice/tutorials#565 |
Merging this PR and continuing testing precice/tutorials#565 for correctness. The changes in this PR make sense. Minor documentation polishing still needs to be done, but this can also be done on the main branch directly. |
Description
Changes:
Testing:
Additional Notes: