LLNL / libROM

Model reduction library with an emphasis on large scale parallelism and linear subspace methods
https://www.librom.net
Other
198 stars 36 forks source link

Changes in Gram-Schmidt functions #239

Closed cval26 closed 7 months ago

cval26 commented 10 months ago

This PR includes two main changes.

  1. Fixes issues in the current orthogonalize method of the Matrix class.
  2. Adds an incremental Gram-Schmidt function orthogonalize_last, where only the last column of the input matrix is orthonormalized, assuming the previous columns are already orthonormal.

The above changes are motivated by applications in the Laghos hydrodynamics code.

dylan-copeland commented 10 months ago

The style script needs to be run.

cval26 commented 10 months ago

@dylan-copeland I just did. Sorry about that; didn't run it right the first time.

cval26 commented 10 months ago

The tests I added in the Matrix test suite fail in the CI build, even though they run successfully in my local build on LC. I'll have to check why and get back to you.

@dylan-copeland Looking at the CI output, it looks like the tests are run once successfully, then run once again and they fail. Is this somehow related to the warning at the beginning of the test output file saying that two active threads have been detected? I'm not sure if the problem is in the unit tests themselves or in the way they were run.

EDIT: The issue has been resolved by the changes made after review number 1.

cval26 commented 10 months ago

@dylan-copeland Thank you for the review comments; the testing issue has been resolved.

@dylan-copeland @chldkdtn Please review the code once more so that we can merge it.

dylan-copeland commented 9 months ago

git pull origin master will resolve the conflicts and should make the regression tests pass (currently they are using wrong command line flags).