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

Phase-field model for fracture using MPI and AMR #203

Merged
merged 35 commits into from
Jan 9, 2025

Conversation

WasimNiyazMunshi
Copy link
Contributor

This pull request adds a phase-field solver for fracture to the code gallery. The fracture model uses the MPI and Adaptive Mesh Refinement (AMR) capabilities of deal.II to solve large scale problems in 3D.

@bangerth
Copy link
Member

bangerth commented Nov 2, 2024

@WasimNiyazMunshi is a visiting student in my lab, and I've contributed to this program. I would appreciate if someone else would handle this PR!

@blaisb blaisb self-requested a review November 3, 2024 17:40
@marcfehling marcfehling self-requested a review November 26, 2024 14:50
Copy link
Member

@marcfehling marcfehling left a comment

Choose a reason for hiding this comment

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

Nice work. The code looks fine! I did not check the formulas in the code, as they overstep my expertise. According to your manuscript, you validated your code against literature data, which suffices for me.

The following two points are more suggestions than requirements for being merged. Please apply according to your own assessment :)

  • If you'd like, you can run a code indentation tool like clang-format over your source file, which further improves readability.
  • If you are feeling fancy: Consider collecting all parameters in a parameter class or multiple classes, and use the ParameterHandler.

Phase_field_fracture_model_in_3D/CMakeLists.txt Outdated Show resolved Hide resolved
Phase_field_fracture_model_in_3D/README.md Outdated Show resolved Hide resolved
Phase_field_fracture_model_in_3D/README.md Outdated Show resolved Hide resolved
Phase_field_fracture_model_in_3D/CMakeLists.txt Outdated Show resolved Hide resolved
Phase_field_fracture_model_in_3D/phase_field.cc Outdated Show resolved Hide resolved
Phase_field_fracture_model_in_3D/phase_field.cc Outdated Show resolved Hide resolved
Phase_field_fracture_model_in_3D/phase_field.cc Outdated Show resolved Hide resolved
Phase_field_fracture_model_in_3D/phase_field.cc Outdated Show resolved Hide resolved
WasimNiyazMunshi and others added 7 commits December 2, 2024 15:32
Removed the *else break* line in the run function. Also moved incrementing the iteration counter to the end of the while loop.
Make the use of template arguments look more uniform.
Use LA::MPI::Vector instead of TrilinosWrappers.
Use FEValues::get_function_values().
Copy link
Member

@blaisb blaisb left a comment

Choose a reason for hiding this comment

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

Just a few comments there and there. Very cool work!

Phase_field_fracture_model_in_3D/README.md Show resolved Hide resolved
Phase_field_fracture_model_in_3D/doc/polyhedral_setup.png Outdated Show resolved Hide resolved
Phase_field_fracture_model_in_3D/phase_field.cc Outdated Show resolved Hide resolved
Phase_field_fracture_model_in_3D/phase_field.cc Outdated Show resolved Hide resolved
Phase_field_fracture_model_in_3D/phase_field.cc Outdated Show resolved Hide resolved
Phase_field_fracture_model_in_3D/phase_field.cc Outdated Show resolved Hide resolved
Phase_field_fracture_model_in_3D/phase_field.cc Outdated Show resolved Hide resolved
@WasimNiyazMunshi
Copy link
Contributor Author

@blaisb, @Rombur and @marcfehling.
Thank you all for taking the time to review my code. I have worked on addressing most of your comments and suggestions. The only pending items are the two blocks of commented-out code that @marcfehling suggested removing. May I kindly request you to review the code once more, so we can proceed with adding it to the code gallery soon? :-)
Best regards
Wasim

@bangerth
Copy link
Member

As for the commented-out code blocks: @WasimNiyazMunshi suggests to leave them in there because many benchmarks are described with traction boundary conditions rather than displacements, and the commented out code is what you will need if you want to use the code for a traction-based modification. That seems reasonable to me.

@WasimNiyazMunshi
Copy link
Contributor Author

WasimNiyazMunshi commented Dec 16, 2024 via email

@marcfehling
Copy link
Member

marcfehling commented Jan 7, 2025

The only pending items are the two blocks of commented-out code that @marcfehling suggested removing.

I'm fine with leaving the commented blocks of code in the patch. The comments you placed in front of the code blocks are helpful for context.

@marcfehling
Copy link
Member

You now have a name conflict: The name PF was already used in petsc. Your namespace PF only contains the PhaseField class -- I would suggest to remove your namespace. @WasimNiyazMunshi @bangerth -- What do you think?

[ 50%] Building CXX object CMakeFiles/phase_field.dir/phase_field.cc.o
/__w/code-gallery/code-gallery/Phase_field_fracture_model_in_3D/phase_field.cc:60:11: error: ‘namespace PF { }’ redeclared as different kind of entity
   60 | namespace PF
      |           ^~
In file included from /usr/include/petsc/petscdmda.h:6,
                 from /usr/include/petsc/petsc.h:14,
                 from /usr/local/include/deal.II/fe/fe_values.h:53,
                 from /__w/code-gallery/code-gallery/Phase_field_fracture_model_in_3D/phase_field.cc:40:
/usr/include/petsc/petscpf.h:36:23: note: previous declaration ‘typedef struct _p_PF* PF’
   36 | typedef struct _p_PF* PF;
      |                       ^~
/__w/code-gallery/code-gallery/Phase_field_fracture_model_in_3D/phase_field.cc: In function ‘int main(int, char**)’:
/__w/code-gallery/code-gallery/Phase_field_fracture_model_in_3D/phase_field.cc:1437:23: error: ‘PF’ is not a namespace-name
 1437 |       using namespace PF;
      |                       ^~
make[2]: *** [CMakeFiles/phase_field.dir/build.make:76: CMakeFiles/phase_field.dir/phase_field.cc.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:90: CMakeFiles/phase_field.dir/all] Error 2
make: *** [Makefile:91: all] Error 2

@bangerth
Copy link
Member

bangerth commented Jan 8, 2025

We use namespaces in all tutorial programs. Perhaps keep it the same here? Maybe just rename it to namespace FracturePropagation or some such?

@WasimNiyazMunshi
Copy link
Contributor Author

WasimNiyazMunshi commented Jan 8, 2025 via email

@marcfehling marcfehling merged commit 8180094 into dealii:master Jan 9, 2025
3 checks passed
@WasimNiyazMunshi
Copy link
Contributor Author

WasimNiyazMunshi commented Jan 9, 2025 via email

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

Successfully merging this pull request may close these issues.

4 participants