Repository navigation
Fix: allow DSP builds without C_PACK by falling back to the configured LAPACK provider - #8258
Open
DanielZhao0432 wants to merge 3 commits into
Open
DanielZhao0432 wants to merge 3 commits into
DanielZhao0432 wants to merge 3 commits into
Conversation
Since deepmodeling#8097, USE_DSP hard-requires MT_CPACK_DIR and DiagoLapack always calls cpack_zhegvx_driver under __DSP, coupling every DSP build to C_PACK even though only the serial generalized eigensolver uses it. Make MT_CPACK_DIR optional: when set, link libc_pack.a as before; when unset, compile the new cpack_zhegvx_fallback.cpp, which provides cpack_zhegvx_driver by forwarding to the reference zhegvx_. The two implementations are selected by CMake source-file selection, so no new compile-time macro is introduced and they can never be linked together. Fixes deepmodeling#8247
…ck case - MODULE_HSOLVER_LCAO compiles diago_lapack.cpp directly, so DSP builds without C_PACK need cpack_zhegvx_fallback.cpp there too. - Add a complex ks_solver=lapack case to the DiagoKPointsTest instantiation; the complex LAPACK path (zhegvx_once) had no registered coverage. - Wording: zhegvx_ comes from the configured LAPACK provider, not necessarily Netlib reference LAPACK.
ABACUS-PR-BOT
left a comment
Collaborator
There was a problem hiding this comment.
No confirmed implementation defect found in the inspected forwarding and build-selection paths. The inline comment asks for a reproducible validation record and clearer skip counts.
Static review of 073b3ae6 against d20090cc: whitespace and governance checks completed with no blockers; the documentation warning was assessed against the stated scope. No build or runtime checks were performed. Full DSP validation remains outstanding as noted in the PR.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reminder
AGENTS.mdanddocs/developers_guide/agent_governance.md.source/changes.Linked Issue
Fix #8247
Unit Tests and/or Case Tests for my changes
cpack_zhegvx_fallback.cppwith-D__DSPtogether with a small driver thatcalls
cpack_zhegvx_driveron a 2x2 generalized eigenproblem (workspacequery + solve, same protocol as
diago_lapack.cpp), linked against systemOpenBLAS/LAPACK → correct eigenvalues, max residual 4.4e-16.
USE_DSP=ONwithoutMT_CPACK_DIR→ configures; prints the fallbackSTATUS message;
cpack_zhegvx_fallback.cppis added to the hsolver target;USE_DSP=ONwith validMT_CPACK_DIR→ configures; fallback not added,libc_pack.alinked (behavior unchanged from [Feat] Add DSP Lapack support #8097);USE_DSP=ONwith a badMT_CPACK_DIR→ FATAL_ERROR with a clear message;MT_CPACK_DIRswitches the provider selection correctly both ways;
ENABLE_LCAO=OFF, noMT_CPACK_DIR) → configures, andthe fallback is correctly not added to any target.
MODULE_HSOLVER_LCAOunit test (ENABLE_ELPA=OFFbuild), now with aregistered complex
ks_solver=lapackcase: 5/5 pass single-rank and on 2 MPIranks. The ELPA variant fails to link on this machine due to a pre-existing
local ELPA/genelpa version mismatch, unrelated to this PR (
diago_elpa.cppis untouched).
python3 tools/03_code_analysis/agent_governance_check.py --staged→ exit 0.073b3ae606).g++ -std=c++11 -D__DSP -I source driver.cpp source/source_hsolver/cpack_zhegvx_fallback.cpp -o test -lopenblas -lgfortran;the driver calls
cpack_zhegvx_driveron a 2x2 generalized eigenproblem(workspace query + solve) and checks residuals of
H z = lambda S z.cmake -S . -B <dir> -DUSE_DSP=ON -DENABLE_ELPA=OFF -DMTBLAS_FFT_DIR=<mock> -DMT_HOST_DIR=<mock> [+- -DMT_CPACK_DIR=<mock>]forthe unset/valid/invalid-prefix, reconfiguration-toggle, and
ENABLE_LCAO=OFFscenarios; provider selection verified via the generated hsolver build files.
ENABLE_ELPA=OFFand runOMP_NUM_THREADS=1 ./MODULE_HSOLVER_LCAO(1 rank) andmpirun -np 2 ./MODULE_HSOLVER_LCAO. Note: the new complex lapack case isNOT skipped on 2 ranks because
nb2d=1gives a 1x1 BLACS grid (dsize==1);the
dsize > 1skip guard only triggers for larger grids.will be verified there (both configurations: with and without
MT_CPACK_DIR)and reported; CI has no DSP toolchain.
What's changed?
MT_CPACK_DIR) becauseDiagoLapackalways callscpack_zhegvx_driverunder__DSP. This makesC_PACK optional: with
MT_CPACK_DIRset, behavior is unchanged; without it,the new
source/source_hsolver/cpack_zhegvx_fallback.cppprovidescpack_zhegvx_driverby forwarding to the configured LAPACK provider'szhegvx_.added to the hsolver objects only when
USE_DSPis on andMT_CPACK_DIRisunset), so no new compile-time macro is introduced and the two implementations
can never be linked together.
MODULE_HSOLVER_LCAOcompilesdiago_lapack.cppdirectly, so the fallback iswired into that test target as well for DSP builds without C_PACK.
ks_solver=lapackcase is added to theDiagoKPointsTestinstantiation in
test_diago_lcao.cpp; the complex LAPACK path previously hadno registered coverage there.
DSP builds (no
__DSPthere).MT_CPACK_DIRremains the way to enable the DSP-offloaded eigensolver. No INPUT changes.
Governance Notes
runtime code path changes for non-DSP builds.