NEGF: New method to extract the matrix Hamiltonians for electrodes and minor changes.#4520
NEGF: New method to extract the matrix Hamiltonians for electrodes and minor changes.#4520hfp merged 1 commit intocp2k:masterfrom
Conversation
|
Sorry, I do not understand the problem in Precommit test. |
|
Please run |
fstein93
left a comment
There was a problem hiding this comment.
Thank you for your contributionbut regarding the old code, I prefer adding a keyword to switch between old and new code or remove the old code entirely.
src/negf_atom_map.F
Outdated
| !> * 08.2017 created [Sergey Chulkov] | ||
| !> * 10.2025 Centering of contact coordinates is added in the end. It is necessary to keep the | ||
| !> right order of indices in the real space image matrices. It is made only here, because | ||
| !> the mapping is based on the comparison ot the coordinates. [Dmitry Ryndyk] |
src/negf_env_types.F
Outdated
| qs_env_contact=qs_env_contact, matrix_s_device=matrix_s_kp(1, 1)%matrix) | ||
| qs_env_contact=qs_env_contact) | ||
| ! CALL negf_env_contact_init_matrices_old(contact_env=negf_env%contacts(icontact), sub_env=sub_env, & | ||
| ! qs_env_contact=qs_env_contact, matrix_s_device=matrix_s_kp(1, 1)%matrix) |
There was a problem hiding this comment.
Is this intended to be kept? If yes, I am in favor of wrapping this using IF (.FALSE.) THEN ... END IF
There was a problem hiding this comment.
Yes, I need it at least for some time. I did not want to make a switch, but OK, I will do it.
By the way, should I make a new pull request or edit this one somehow?
Thank you for the help.
There was a problem hiding this comment.
Just edit this one.
In general, a (temporary) switch is better than asking the user to recompile because there is more control from the compiler and an actual way to still test the old code using the regression tests without the inconvenient way of recompilation especially if this temporary solution persits longer. What is your timeline on replacing the old code?
There was a problem hiding this comment.
Actually, I did not plan to inform usual users about it; it is only for developers, e.g., only me at the moment. )
I will remove it as soon as I am sure that the new code is perfect.
There was a problem hiding this comment.
I removed the old code; better to keep only in my repository.
I made it before and repeated it now. make -j pretty |
Despite of status OK, you may need to check like |
a057102 to
85cb861
Compare
fstein93
left a comment
There was a problem hiding this comment.
Thank you for considering the suggestions.
|
LGTM as well. I started the optional Regtest psmp (we can ignore the current breakage). |
Passed, hence merging the PR. |
No description provided.