yuanming-hu / taichi_mpm

High-performance moving least squares material point method (MLS-MPM) solver. (ACM Transactions on Graphics, SIGGRAPH 2018)
MIT License
2.34k stars 317 forks source link

Fix 88-Line MPM ambiguity #19

Open yuanming-hu opened 5 years ago

yuanming-hu commented 5 years ago
i have a question about mpm88
line 97 in mls-mpm88-explained.cpp: https://github.com/yuanming-hu/taichi_mpm/blob/master/mls-mpm88-explained.cpp
should there be an identity matrix after (J-1)*J?
sethlu commented 5 years ago

I think the code is a little ambiguous as a temporary diagonal taichi::MatrixND is initialized from (lambda * (J-1) * J) with the following implicit constructor:

  TC_FORCE_INLINE MatrixND(T v) : MatrixND() {
    for (int i = 0; i < dim; i++) {
      d[i][i] = v;
    }
  }

Then the following operator override is used to carry out the matrix-matrix addition:

  TC_FORCE_INLINE MatrixND operator+(const MatrixND &o) const {
    return MatrixND([=](int i) { return this->d[i] + o[i]; });
  }

🤔 For readability it may be nice to initialize the diagonal matrix explicitly?

superdump commented 4 years ago

This really confused me - it looked like the line was trying to add a scalar to a matrix.

Also, it is not clear how this matches equation 52 here: https://www.seas.upenn.edu/~cffjiang/research/mpmcourse/mpmcourse.pdf . That equation has F^-T in the second term, which I am guessing is meant to mean the transpose of the inverse matrix of F? In the implementation, the first term is multiplied by F^T (the transpose of F). How is this correctly implemented?

Please make the code explicit for readability instead of using an implicit constructor.