Skip to content

Conversation

@fstein93
Copy link
Contributor

@fstein93 fstein93 commented Nov 21, 2022

This PR enforces the block size to be a power of 2 (also on CPU) if an eigenvalue decomposition with ELPA is requested.

@oschuett
Copy link
Member

It would be awesome if this fixes #464!

@fstein93 fstein93 force-pushed the ELPA branch 2 times, most recently from 336473d to 45eba93 Compare November 22, 2022 12:08
@fstein93
Copy link
Contributor Author

I have rerun some of the failing calculations on Daint and obtained the correct results. Do you think it is reasonable to employ Scalapack instead of ELPA in all failing tests? I know for sure that some passing tests (mostly from RPA/MP2) definitely make use of eigenvalue decompositions.

@oschuett
Copy link
Member

Do you think it is reasonable to employ Scalapack instead of ELPA in all failing tests?

No, we should really get to the bottom of this. Otherwise I'd have very little trust in the results - despite some tests occasionally passing. Fortunately, most of the tests failed because ELPA itself returned an error code. So, those should be rather straightforward to investigate.

@oschuett
Copy link
Member

The timeouts are probably due to marekandreas/elpa#17.
I guess, until this point we never actually used ELPA's GPU kernels.

@fstein93
Copy link
Contributor Author

Apart from two special cases which require results by other regtests and the timeouts, there is only one kind of error in the bandred code of ELPA. I will consult the ELPA developers for that. It might take some time.

@fstein93
Copy link
Contributor Author

Apparently, the error is related to the 2stage solver because we observe timeouts if we employ the 1stage solver instead.

@fstein93 fstein93 changed the title Fix compilations issues and blocksizes in Elpa Fix blocksizes in ELPA Dec 4, 2022
@oschuett
Copy link
Member

It seems #2447 fixed the timeouts. @fstein93, could you please revert back to the two-stage solver?

@fstein93
Copy link
Contributor Author

Apparently, everything works now. If there are no further issues raised, I will merge this PR.

@fstein93 fstein93 merged commit 0a22203 into cp2k:master Dec 13, 2022
@oschuett
Copy link
Member

I'm a bit disappointed that we don't see any improvements in our performance test.
Shouldn't this show up in the diag_cu144_broy.inp benchmark?

@fstein93
Copy link
Contributor Author

Might be but I don't know what kernel we had used before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants