Skip to content

Conversation

@bodhinandach
Copy link
Member

Describe the PR
This PR modifies the assembly procedure of sparse matrices from direct memory access to tripletList. TripletList is conceptually a vector of Eigen::Triplet<double> which contains 2 indices and a double.

Related Issues/PRs
#4, #5

Additional context
This construction can improve the simulation time 1.5 times faster in ImplicitLinear solver. Though I need to run some benchmarks for other multi-phase solvers.

@bodhinandach bodhinandach added the enhancement New feature or request label Oct 19, 2021
@bodhinandach bodhinandach self-assigned this Oct 19, 2021
@bodhinandach
Copy link
Member Author

bodhinandach commented Oct 19, 2021

@RyotaHashimoto Can you help me to confirm if this is faster compare to the current code?

Copy link
Collaborator

@RyotaHashimoto RyotaHashimoto left a comment

Choose a reason for hiding this comment

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

The changes look good to me.
I checked the performance of the code for the 3D beam problem using the linear shape function (2 MPI ranks and 8 threads are used) as well as the code review.
The older code took about 42000ms, and the new code took about 24000ms.
So the new code was 1.75 times faster than the old one.
Great job, @bodhinandach !

@bodhinandach
Copy link
Member Author

Thanks for confirming @RyotaHashimoto. Please don't merge this branch as I am still checking the performance of other solvers.

@codecov-commenter
Copy link

Codecov Report

Merging #16 (0888d17) into master (d31f823) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #16   +/-   ##
=======================================
  Coverage   95.23%   95.23%           
=======================================
  Files         185      185           
  Lines       36713    36716    +3     
=======================================
+ Hits        34960    34963    +3     
  Misses       1753     1753           
Impacted Files Coverage Δ
...ers/assemblers/assembler_eigen_implicit_linear.tcc 78.48% <100.00%> (+0.85%) ⬆️
include/solvers/mpm_implicit_linear.tcc 83.64% <100.00%> (ø)

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 d31f823...0888d17. Read the comment docs.

@bodhinandach
Copy link
Member Author

Let's merge this. The Navier-Stokes and Two-Phase assemblers do not work better with this modification. Maybe there is a size effect of the matrix assembly.

@bodhinandach bodhinandach merged commit 5fe982d into master Oct 20, 2021
@bodhinandach bodhinandach deleted the feature/fast_matrix_assembly branch October 20, 2021 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants