Fix: Remove the C++17 feature induced by #7588 and update comments#7619
Closed
maki49 wants to merge 2 commits into
Closed
Fix: Remove the C++17 feature induced by #7588 and update comments#7619maki49 wants to merge 2 commits into
maki49 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refines the EXX-related H/dH matrix output paths introduced in #7588 by removing a C++17-only construct and making the gamma-only limitation explicit (both in comments and runtime behavior) to avoid silently missing EXX contributions.
Changes:
- Replaces
if constexpr(C++17) in the LCAO SCF controller with a C++11/14-compatible overload-based dispatch for wiring gamma-only EXX interfaces. - Adds explicit runtime rejection (
WARNING_QUIT) for unsupported multi-k + EXX combinations forout_mat_h_exx, and forout_mat_dh/out_mat_dh_exxwhen EXX is on. - Updates inline documentation to clarify why multi-k EXX output is not supported for these writers and why the interfaces remain gamma-only.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| source/source_io/module_hs/write_H_terms.h | Clarifies gamma-only EXX interface intent for WriteHParams. |
| source/source_io/module_hs/write_H_terms.cpp | Adds explicit quit for out_mat_h_exx when not gamma-only; includes quit helper header. |
| source/source_io/module_dhs/write_dH.h | Expands rationale for keeping EXX interfaces gamma-only in WriteDHParams. |
| source/source_io/module_dhs/write_dH.cpp | Adds explicit quit for multi-k + EXX when out_mat_dh/out_mat_dh_exx are requested. |
| source/source_io/module_ctrl/ctrl_scf_lcao.cpp | Removes C++17 if constexpr by introducing overload-based EXX interface wiring for gamma-only. |
Comment on lines
+148
to
+149
| if (GlobalC::exx_info.info_global.cal_exx && !PARAM.globalv.gamma_only_local | ||
| && (PARAM.inp.out_mat_dh[0] || PARAM.inp.out_mat_dh_exx[0])) |
Comment on lines
+409
to
+416
| // The EXX interfaces carried by WriteHParams are gamma-only (see write_H_terms.h), so at | ||
| // multi-k exd/exc are both null. Quit with a clear message instead of writing nothing. | ||
| if (!PARAM.globalv.gamma_only_local) | ||
| { | ||
| ModuleBase::WARNING_QUIT("write_h_exx", | ||
| "out_mat_h_exx is only supported for gamma-only: the V^EXX(R) " | ||
| "output is not available at multi-k. Use gamma_only."); | ||
| } |
Comment on lines
436
to
442
| if (inp.out_mat_h_exx[0] && GlobalC::exx_info.info_global.cal_exx) | ||
| { | ||
| // V^EXX(R) output is wired for the gamma (TK==double) exx interfaces. | ||
| if constexpr (std::is_same<TK, double>::value) | ||
| { | ||
| if (GlobalC::exx_info.info_global.cal_exx) | ||
| { | ||
| if (exx_nao.exd) { h_params.exd = exx_nao.exd.get(); } | ||
| if (exx_nao.exc) { h_params.exc = exx_nao.exc.get(); } | ||
| ModuleIO::write_h_exx(h_params); | ||
| } | ||
| } | ||
| // V^EXX(R) output is wired for the gamma (TK==double) exx interfaces; at multi-k | ||
| // this leaves exd/exc null and write_h_exx quits with an explicit message. | ||
| set_exx_interfaces(h_params, exx_nao); | ||
| ModuleIO::write_h_exx(h_params); | ||
| } |
Collaborator
|
This has been solved in another PR, if you have new updates, you can update the code based on the new one. |
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.
Refine #7588: (the PR for dH feature)
constexpr