Skip to content

Fix(hsolver): add early PW basis-size guard to report npw<nbands clearly #7237

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.

Sign up for GitHub

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

Closed
Cstandardlib wants to merge 1 commit into from
+19 −3
Closed

Fix(hsolver): add early PW basis-size guard to report npw<nbands clearly #7237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion source/source_cell/check_atomic_stru.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void check_atomic_stru(UnitCell& ucell, const double& factor)
else
{
std::stringstream mess;
mess << "Notice: symbol '" << symbol1 << "' is not an element symbol!!!! ";
mess << " Notice: symbol '" << symbol1 << "' is not an element symbol!!!! ";
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

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

This whitespace-only change to the emitted warning text (adding a leading space before "Notice") is unrelated to the PR’s stated purpose (HSolverPW early basis-size guard). If it’s intentional, it should be called out in the PR description; otherwise consider reverting it to keep the change set focused.

Copilot uses AI. Check for mistakes.
mess << "set the covalent radius to be 0." << std::endl;
GlobalV::ofs_running << mess.str();
std::cout << mess.str();
Expand Down
20 changes: 18 additions & 2 deletions source/source_hsolver/hsolver_pw.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,22 @@ void HSolverPW<T, Device>::hamiltSolvePsiK(hamilt::Hamilt<T, Device>* hm,
#endif

const int cur_nbasis = psi.get_current_nbas();
const int dim = psi.get_current_ngk();
const int nband = psi.get_nbands();

// Guard against under-complete PW basis at current k-point.
// This case is common in highly compressed cells with low ecutwfc,
// and can otherwise fail later with obscure solver-specific messages.
if (dim < nband)
Comment on lines +254 to +257
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

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

The new under-complete-basis guard introduces an important new failure mode/message, but there doesn’t appear to be a unit test exercising HSolverPW::hamiltSolvePsiK for npw_k < nbands. If feasible, add a small gtest that constructs a Psi with ngk[k]=dim<nbands and asserts the code aborts with the expected message (or throws in UT builds) to prevent regressions.

Copilot uses AI. Check for mistakes.
{
Comment on lines +251 to +258
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

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

The guard compares dim = psi.get_current_ngk() (which appears to be the local number of plane waves on the current rank after PW distribution) against nband. In MPI runs it’s common for a rank’s local npw to be < nbands even when the global PW dimension (sum over POOL_WORLD) is sufficient, so this can abort valid parallel calculations. Consider reducing dim over POOL_WORLD (sum) to a global npw_k for the current k-point and compare that against nband (and optionally print both local and global dims).

Copilot uses AI. Check for mistakes.
std::cout << "\n ERROR in HSolverPW::hamiltSolvePsiK" << std::endl;
std::cout << " solver = " << this->method << std::endl;
std::cout << " ik (1-based) = " << nk_nums + 1 << std::endl;
std::cout << " npw(dim) = " << dim << std::endl;
Comment on lines +260 to +262
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

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

ik (1-based) = nk_nums + 1 is misleading: nk_nums is passed as this->wfc_basis->nks (total k-points in the pool) at the call sites, not the current k-point index. Prefer using psi.get_current_k() (after psi.fix_k(ik)) or pass the actual ik into hamiltSolvePsiK, otherwise the reported k-point will be wrong.

Copilot uses AI. Check for mistakes.
std::cout << " nbands = " << nband << std::endl;
std::cout << " Suggestion : increase ecutwfc, reduce nbands, or adjust k-mesh." << std::endl;
ModuleBase::WARNING_QUIT("HSolverPW::hamiltSolvePsiK", "insufficient plane-wave basis dimension for requested nbands");
Comment on lines +259 to +265
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

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

The detailed diagnostics are printed via multiple std::cout lines, but WARNING_QUIT only records a short message in warning.log (rank 0) and will also print its own standardized NOTICE block. To keep the actionable details in logs and avoid duplicated multi-rank stdout spam, consider composing a single descriptive message (e.g., with std::stringstream) and passing it to WARNING_QUIT, and/or gate extra stdout logging to GlobalV::MY_RANK==0.

Copilot uses AI. Check for mistakes.
}

// Shared matrix-blockvector operators used by all iterative solvers.
auto hpsi_func = [hm, cur_nbasis](T* psi_in, T* hpsi_out, const int ld_psi, const int nvec) {
Expand Down Expand Up @@ -288,8 +304,8 @@ void HSolverPW<T, Device>::hamiltSolvePsiK(hamilt::Hamilt<T, Device>* hm,
cg.diag(hpsi_func,
spsi_func,
psi.get_nbasis(),
psi.get_nbands(),
psi.get_current_ngk(),
nband,
dim,
psi.get_pointer(),
eigenvalue,
this->ethr_band,
Expand Down
Loading