cb-geo / mpm

CB-Geo High-Performance Material Point Method
https://www.cb-geo.com/research/mpm
Other
238 stars 83 forks source link

Refactor/mpm solvers to handle different stress update schemes #685

Closed kks32 closed 3 years ago

kks32 commented 3 years ago

Describe the PR Replacing multiple if-else branching conditions for solver types and interface with a new StressUpdate and Interface class. This design will use the Factory pattern to handle different StressUpdate types and Interface.

Performance measurement on Stampede2

Performance tests were conducted for 3D hydrostatic column with USF 10,000 steps Refactored: 731639 ms Dev version: 731874 ms There is no perceivable difference between the two versions in terms of performance.

This PR is a work in progress that addresses #686

Additional context

Related Issues/PRs An RFC is available at #686

codecov[bot] commented 3 years ago

Codecov Report

Merging #685 into develop will increase coverage by 0.13%. The diff coverage is 92.64%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #685      +/-   ##
===========================================
+ Coverage    96.67%   96.81%   +0.13%     
===========================================
  Files          123      131       +8     
  Lines        25560    25811     +251     
===========================================
+ Hits         24710    24987     +277     
+ Misses         850      824      -26     
Impacted Files Coverage Δ
include/io/io.h 100.00% <ø> (ø)
include/solvers/mpm.h 100.00% <ø> (ø)
include/solvers/mpm_scheme/mpm_scheme.tcc 67.95% <67.95%> (ø)
include/solvers/mpm_scheme/mpm_scheme_usl.tcc 77.78% <77.78%> (ø)
include/solvers/mpm_base.tcc 76.67% <91.07%> (+0.58%) :arrow_up:
include/solvers/mpm_explicit.tcc 95.08% <92.86%> (+31.88%) :arrow_up:
include/contacts/contact.h 100.00% <100.00%> (ø)
include/contacts/contact.tcc 100.00% <100.00%> (ø)
include/contacts/contact_friction.tcc 100.00% <100.00%> (ø)
include/solvers/mpm_base.h 50.00% <100.00%> (+50.00%) :arrow_up:
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 44479a6...2a7aca1. Read the comment docs.

kks32 commented 3 years ago

Generic factory design isn't efficient as it involves separately initializing 2D and 3D StressUpdate classes. The current if-else in the constructor is sufficient.

kks32 commented 3 years ago

This refactoring doesn't remove pressure_smoothing and compute_stress_strain in solver class, exactly for this reason that it doesn't break @tianchiTJ and @bodhinandach changes. This shouldn't affect your derived classes and can stay the same. The SemiImplicit and TwoPhase can be refactored later once they are merged to develop. Let me know if this is still an issue.

This change affects the Contact so @thiagordonho has to change the interface class

bodhinandach commented 3 years ago

@kks32 pressure_smoothing, compute_stress_strain, and the prior stress_update_ are the three things we need, I guess. The last part may be an issue for the two_phase branch.

kks32 commented 3 years ago

@tianchiTJ @bodhinandach I have reverted stress_update_ so you can use the variable as before. However, the type is a string and not an enum. So instead of mpm::StressUpdate::USF use usf and instead of mpm::StressUpdate::USL use usl. Thanks. Let me know if there are any issues.

bodhinandach commented 3 years ago

I would like to recommend everyone @cb-geo/mpm to look at this PR too. I guess it is a good direction for everyone. @kks32 Can we merge this after next week's group meeting if that's okay?

kks32 commented 3 years ago

Thanks @kks32. In terms of big picture thought process then, we have the following:

  • MPMExplicit as the MPM type that defines the solution scheme and the phase

  • MPMScheme as the detail within the type which houses the stress update scheme, even what to do with throw elements - in the future I want to get into more integration schemes.

  • Analysis would be separate from the MPMScheme then? How does the JSON look like?

Nothing changes in the JSON.