Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Wannier localization when using LOW SPIN ROKS #3108

Merged
merged 5 commits into from
Nov 16, 2023

Conversation

IlyaFed
Copy link
Contributor

@IlyaFed IlyaFed commented Nov 13, 2023

  • Added the ability to calculate Wannier Сenters for the ROKS method (only LOCALIZE NONE and JACOBI).
  • Added the error message when ROKS and LOCALIZE were used incorrectly (instead of Segmentation Fault).
  • Replaced the output of two spins in ROKS by the output of spin 1 only, since the densities of spin_1/2 are the same.

I have previously raised this issue in the Google Group discussion https://groups.google.com/g/cp2k/c/yJPJS2vUkBI

This Wannier centres analysis is added in analogy to the implementation in CPMD code. See https://github.com/CPMD-code/CPMD/blob/main/src/ddipo_utils.mod.F90, The fragment “ELSEIF (lspin2%tlse) THEN…” means that for singly occupied orbitals the Jacobi rotation is disabled. In this case, the number of orbitals to which the Jacobi rotation can be applied has been reduced by 2 since SOMO-1 and SOMO-2 orbitals should not rotate.

- Added the ability to calculate Wannier Сenters for the ROKS method (only LOCALIZE NONE and JACOBI).
- Added the error message when ROKS and LOCALIZE were used incorrectly (instead of Segmentation Fault).
- Replaced the output of two spins in ROKS by the output of spin 1 only, since the densities of spin_1/2 are the same.

I have previously raised this issue in the Google Group discussion https://groups.google.com/g/cp2k/c/yJPJS2vUkBI

This Wannier centres analysis is added in analogy to the implementation in CPMD code.
See https://github.com/CPMD-code/CPMD/blob/main/src/ddipo_utils.mod.F90,
The fragment “ELSEIF (lspin2%tlse) THEN…” means that for singly filled orbitals the Jacobi rotation is disabled.
In this case, the number of orbitals to which the Jacobi rotation can be applied has been reduced by 2 since SOMO-1 and SOMO-2 orbitals should not rotate.
@@ -365,6 +370,11 @@ SUBROUTINE jacobi_rotations_serial(weights, zij, vectors, max_iter, eps_localiza
CALL cp_fm_get_info(rmat, nrow_global=nstate)
tolerance = 1.0e10_dp

IF (restricted > 0) THEN
WRITE (6, *) "JACOBI: The last ", restricted, " orbitals do not rotate"
Copy link
Member

Choose a reason for hiding this comment

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

Note CP2K's coding convention #c012 concerning hardwired unit numbers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I missed it. Replaced it in the code with cp_logger_get_default_unit_nr().

@@ -2208,6 +2220,13 @@ SUBROUTINE jacobi_rot_para(weights, zij, vectors, para_env, max_iter, eps_locali

CALL cp_fm_get_info(rmat, nrow_global=nstate)

IF (restricted > 0) THEN
IF (output_unit > 0) THEN
WRITE (6, *) "JACOBI: The last ", restricted, " orbitals do not rotate"
Copy link
Member

Choose a reason for hiding this comment

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

Same, see here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I missed it. Replaced it in the code with cp_logger_get_default_io_unit().

@mkrack
Copy link
Member

mkrack commented Nov 14, 2023

Thanks for your contribution. Could you also add a regression test input for that new feature?

@IlyaFed
Copy link
Contributor Author

IlyaFed commented Nov 15, 2023

I added two tests to QS/regtest-lsroks

The first test checks the output of localized Wannier Centers for ROKS. It would be more correct to check the centers themselves, but as far as I understand, the regression tests do not support this and I added a check for “Total Spread (Berry)”.

The second test checks the MO_CUBES output for ROKS. It would be more correct to check that it outputs cube files only for spin-1, but as I understand it, this is also not supported, so I added a check that the program worked through "ENERGY| Total FORCE_EVAL" and a comment to the input.

@IlyaFed
Copy link
Contributor Author

IlyaFed commented Nov 15, 2023

I'm sorry, could you explain what's wrong with Regtest sdbg, it says that all tests were successful...

@mkrack
Copy link
Member

mkrack commented Nov 15, 2023

I'm sorry, could you explain what's wrong with Regtest sdbg, it says that all tests were successful...

Just one slow test was detected. That can happen sometimes without a serious reason. I requested to re-run the sdbg test.

@IlyaFed
Copy link
Contributor Author

IlyaFed commented Nov 15, 2023

I'm sorry, could you explain what's wrong with Regtest sdbg, it says that all tests were successful...

Just one slow test was detected. That can happen sometimes without a serious reason. I requested to re-run the sdbg test.

Thank you.

@mkrack mkrack merged commit 79e6028 into cp2k:master Nov 16, 2023
34 checks passed
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.

None yet

2 participants