Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a Fluid-Structure Interaction (FSI) solver implementation as part of a project report. The implementation uses the deal.II finite element library to solve coupled fluid-solid problems with interface terms.
Key Changes:
- Complete FSI solver implementation with hp-finite elements
- Custom block preconditioner for the coupled system
- Test driver program for 2D/3D FSI problems
Reviewed changes
Copilot reviewed 37 out of 54 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test.cpp | Simple test driver that initializes MPI and runs FSI simulation |
| src/FSI.hpp | Header file defining FSI class with preconditioner, boundary conditions, and helper functions |
| src/FSI.cpp | Implementation of FSI methods including assembly, solving, mesh refinement, and output |
| img/mesh2.jpeg | Binary image file showing mesh visualization |
| build/* | Build artifacts that should not be in version control |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
build/cmake_install.cmake
Outdated
| # Install script for directory: /home/fefe/Uni/nmpde-FSI | ||
|
|
||
| # Set the install prefix | ||
| if(NOT DEFINED CMAKE_INSTALL_PREFIX) | ||
| set(CMAKE_INSTALL_PREFIX "/usr/local") | ||
| endif() | ||
| string(REGEX REPLACE "/$" "" CMAKE_INSTALL_PREFIX "${CMAKE_INSTALL_PREFIX}") | ||
|
|
||
| # Set the install configuration name. | ||
| if(NOT DEFINED CMAKE_INSTALL_CONFIG_NAME) | ||
| if(BUILD_TYPE) | ||
| string(REGEX REPLACE "^[^A-Za-z0-9_]+" "" | ||
| CMAKE_INSTALL_CONFIG_NAME "${BUILD_TYPE}") | ||
| else() | ||
| set(CMAKE_INSTALL_CONFIG_NAME "Release") | ||
| endif() | ||
| message(STATUS "Install configuration: \"${CMAKE_INSTALL_CONFIG_NAME}\"") | ||
| endif() | ||
|
|
||
| # Set the component getting installed. | ||
| if(NOT CMAKE_INSTALL_COMPONENT) | ||
| if(COMPONENT) | ||
| message(STATUS "Install component: \"${COMPONENT}\"") | ||
| set(CMAKE_INSTALL_COMPONENT "${COMPONENT}") | ||
| else() | ||
| set(CMAKE_INSTALL_COMPONENT) | ||
| endif() | ||
| endif() | ||
|
|
||
| # Install shared libraries without execute permission? | ||
| if(NOT DEFINED CMAKE_INSTALL_SO_NO_EXE) | ||
| set(CMAKE_INSTALL_SO_NO_EXE "1") | ||
| endif() | ||
|
|
||
| # Is this installation the result of a crosscompile? | ||
| if(NOT DEFINED CMAKE_CROSSCOMPILING) | ||
| set(CMAKE_CROSSCOMPILING "FALSE") | ||
| endif() | ||
|
|
||
| # Set default install directory permissions. | ||
| if(NOT DEFINED CMAKE_OBJDUMP) | ||
| set(CMAKE_OBJDUMP "/u/sw/toolchains/gcc-glibc/11.2.0/prefix/bin/objdump") | ||
| endif() | ||
|
|
||
| if(CMAKE_INSTALL_COMPONENT) | ||
| set(CMAKE_INSTALL_MANIFEST "install_manifest_${CMAKE_INSTALL_COMPONENT}.txt") | ||
| else() | ||
| set(CMAKE_INSTALL_MANIFEST "install_manifest.txt") | ||
| endif() | ||
|
|
||
| string(REPLACE ";" "\n" CMAKE_INSTALL_MANIFEST_CONTENT | ||
| "${CMAKE_INSTALL_MANIFEST_FILES}") | ||
| file(WRITE "/home/fefe/Uni/nmpde-FSI/build/${CMAKE_INSTALL_MANIFEST}" | ||
| "${CMAKE_INSTALL_MANIFEST_CONTENT}") |
There was a problem hiding this comment.
The build directory and its contents should not be committed to version control. Build artifacts are generated files that should be excluded using .gitignore. This includes all files in the build/ directory such as CMakeFiles, Makefile, cmake_install.cmake, etc.
There was a problem hiding this comment.
I second this comment: you should not include build artifacts (binary files etc, everything that is inside your build folder) in the git repository, for a variety of reasons. The first few that come to mind:
- the build artifacts will be specific to your computer (and thus most likely useless to anyone that has a different architecture, or dependencies installed in a different place);
- git is not made to track changes in binary files, and this will result in your repository becoming inordinately large.
src/FSI.cpp
Outdated
| system_matrix); | ||
| } | ||
| /** | ||
| * Case 2: The neighbor has further chigit config --global core.editor "vim"ldren. |
There was a problem hiding this comment.
There is a typo in the comment. "chigit config --global core.editor "vim"ldren" should be "children". It appears that a git command was accidentally pasted into the comment.
src/FSI.hpp
Outdated
| tmp_d.sadd(-1.0, 1.0, src.block(2)); // residual = y - B*px | ||
|
|
||
| { | ||
| SolverControl solver_control_disp(2000, 1e-2 *tmp_d.l2_norm()); |
There was a problem hiding this comment.
The variable name tmp_d is not descriptive. Consider renaming to something more meaningful like displacement_residual or tmp_displacement to improve code readability.
| TrilinosWrappers::PreconditionAMG preconditioner_displacement; | ||
|
|
||
| // Temporary vector. | ||
| mutable Vector<double> tmp_p; |
There was a problem hiding this comment.
The variable name tmp_p is not descriptive. Consider renaming to something more meaningful like pressure_residual or tmp_pressure to improve code readability.
| { | ||
| { | ||
| // Solve fluid system | ||
| SolverControl solver_control_vel(2000, 1e-2 * src.block(0).l2_norm()); |
There was a problem hiding this comment.
The deal.II class ReductionControl is actually a slightly better way to specify relative tolerances to linear solvers.
The report is also present here in the README.md file of the repositry.
The group members are:
Paolo Potì, Giulia Cavoletti, Matteo Parimbelli, Federico Pizzolato