Describe the Code Quality Issue
K_Vectors (in the lower-level source/source_cell/ module) decides whether to reduce k-points to the irreducible Brillouin zone (IBZ) by directly depending on the higher-level I/O module source_io/module_unk/berryphase:
// source/source_cell/klist.cpp
#include "source_io/module_unk/berryphase.h"
...
// only berry phase need all kpoints including time-reversal symmetry!
if (!berryphase::berry_phase_flag && ModuleSymmetry::Symmetry::symm_flag != -1)
{
// reduce kpoints to IBZ ...
}
This is problematic for two reasons:
- Wrong dependency direction / layering inversion. source_cell is a low-level, foundational module, yet it #includes and reads the global flag berryphase::berry_phase_flag from source_io. A low-level module should not depend back on an upper-level I/O module.
- Control logic smuggled through a global variable. Whether k-points are reduced to the IBZ is really a decision made by the caller (the esolver) based on input parameters, but here it is passed implicitly through a cross-module global flag. This hurts readability and testability. As a consequence, several unit tests (klist_test.cpp, klist_test_para.cpp, for_testing_klist.h, etc.) are forced to define an unrelated stub bool berryphase::berry_phase_flag = ...; just to link.
Additional Context
No response
Task list for Issue attackers (only for developers)
Describe the Code Quality Issue
K_Vectors (in the lower-level source/source_cell/ module) decides whether to reduce k-points to the irreducible Brillouin zone (IBZ) by directly depending on the higher-level I/O module source_io/module_unk/berryphase:
This is problematic for two reasons:
Additional Context
No response
Task list for Issue attackers (only for developers)