TridiagSolver: fix missing sort in the deflation #960
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cscs-ci run |
albestro
force-pushed
the
fix-tridiag-solver
branch
from
dc7d176
to
f79d0c5
Compare
|
cscs-ci run |
albestro
changed the title
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #960 +/- ##
==========================================
+ Coverage 93.35% 94.83% +1.47%
==========================================
Files 143 129 -14
Lines 8605 7795 -810
Branches 1103 1049 -54
==========================================
- Hits 8033 7392 -641
+ Misses 388 238 -150
+ Partials 184 165 -19
|
albestro
added a commit
that referenced
this pull request
Aug 22, 2023
it was probably unused since #819
albestro
force-pushed
the
fix-tridiag-solver
branch
3 times, most recently
from
c781736
to
1012688
Compare
albestro
force-pushed
the
fix-tridiag-solver
branch
from
1012688
to
fcff6bf
Compare
rasolca
approved these changes
Aug 29, 2023
Co-authored-by: Raffaele Solcà <[email protected]>
Co-authored-by: Raffaele Solcà <[email protected]>
|
cscs-ci run |
TL;DR
Build CP2K
Test convergence H20-128InputFileialberto@daint103:~/workspace/cp2k> git --no-pager diff H2O-128.inp
diff --git a/H2O-128.inp b/H2O-128.inp
index 53bf03706..6b2bb0761 100644
--- a/H2O-128.inp
+++ b/H2O-128.inp
@@ -8,23 +8,21 @@
REL_CUTOFF 30
&END MGRID
&QS
- EPS_DEFAULT 1.0E-12
+ # EPS_DEFAULT 1.0E-12
WF_INTERPOLATION PS
EXTRAPOLATION_ORDER 3
&END QS
&SCF
SCF_GUESS ATOMIC
- &OT ON
- MINIMIZER DIIS
- &END OT
+ &DIAGONALIZATION ON
+ ALGORITHM STANDARD
+ &END DIAGONALIZATION
# SCF_GUESS RESTART
# EPS_SCF 1.0E-7
-
&PRINT
&RESTART OFF
&END
&END
-
&END SCF
&XC
&XC_FUNCTIONAL Pade
@@ -434,14 +432,12 @@
&END FORCE_EVAL
&GLOBAL
PROJECT H2O-128
- RUN_TYPE MD
+ RUN_TYPE ENERGY
PRINT_LEVEL LOW
+ # PREFERRED_DIAG_LIBRARY scalapack
+ PREFERRED_DIAG_LIBRARY dlaf
+ &FM
+ NCOL_BLOCKS 512
+ NROW_BLOCKS 512
+ &END FM
&END GLOBAL
-&MOTION
- &MD
- ENSEMBLE NVE
- STEPS 10
- TIMESTEP 0.5
- TEMPERATURE 300.0
- &END MD
-&END MOTIONScalapack vs DLAF @ PizDaint-MCAll runs converged and all steps reported the same total energy (up to ~1e-13).
Scalapack-192DLAF256 (RPN=9)DLAF1024 (RPN=9)DLAF512 (RPN=2) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(probable fix for #953)
Deflation process might produce changes in the order of deflated eigenvalues, and the part taking care of keeping them sorted was not implemented in our version. So, deflated eigenvalues were not always sorted correctly and this ended up with wrong results or
NaNvalues. (See dlaed2 for more details)Thanks to @RMeli and @rasolca for the investigation and the support in fixing this.
In addition to the bug fix, this introduces also
std::hypotfor avoiding possible numerical errors in the deflation step (it has been preferred thecppstdone over the lapackdlapy2, without any strong reason).Another change is about removal of unused GPU kernels related to this step (namely
stablePartitionIndexOnDeviceand related).TODO:
stablePartitionIndexForDeflationminiapp_tridiag_solver