Repository navigation
Refactor(input): make relax_method a scalar option - #8168
Stardust0831 wants to merge 2 commits into
Conversation
|
Thanks! Maybe it would be better to change 'cg_nested' to 'cg_trad' and change 'bfgs_traditional' to 'bfgs_trad‘? |
|
@ABACUS-PR-BOT Could you review the PR? |
ABACUS-PR-BOT
left a comment
There was a problem hiding this comment.
Static review of 96cca9ce281ce38512c91ec5812f7ac1cbb1baf2: no new correctness issue found in the scalar parsing and relaxation dispatch changes. One optional documentation suggestion below. No build/runtime tests were run; CI diagnostics were unavailable through the required checks connector.
| ## Optimization Algorithms | ||
|
|
||
| ABACUS offers multiple optimization algorithms for structural relaxation, which can be selected using the [relax_method](./input_files/input-main.md#relax_method) keyword. The optional second value selects an implementation variant for CG or BFGS. For both methods, variant 1 is the traditional implementation and variant 2 is the recommended default. | ||
| ABACUS offers multiple optimization algorithms for structural relaxation, which can be selected using the [relax_method](./input_files/input-main.md#relax_method) keyword. Specify one scalar value. |
There was a problem hiding this comment.
Non-blocking: consider adding a short upgrade table here for the finalized scalar names (currently cg 1 -> cg_nested, cg 2 -> cg, bfgs 1 -> bfgs_traditional, bfgs 2 -> bfgs). The old two-token inputs now fail validation, and this mapping is only in the PR description. A documentation follow-up after merge is fine.
|
@mohanchen Thank you for the review and suggestions; I’ve renamed the options to cg_trad and bfgs_trad as requested in a follow-up commit. |
relax_methodnow selects each relaxation method with one scalar string. This removes the algorithm/variant pair from INPUT parsing and the relaxation interfaces, makes availability expressions refer to one well-defined value, and gives invalid inputs a complete list of accepted choices.The
relax_methodvalues change as follows:cgorcg 2cgcg 1cg_tradbfgsorbfgs 2bfgsbfgs 1bfgs_tradcg_bfgscg_bfgs(unchanged)lbfgslbfgs(unchanged)sdsd(unchanged)The default remains simultaneous CG (
cg). All two-token values are rejected; no compatibility aliases are retained.Reminder
AGENTS.mdanddocs/developers_guide/agent_governance.md.source/changes.Linked Issue
Fixes #7822. Also addresses the availability-semantics discussion in #7783.
That discussion identified ambiguity between scalar membership (
in) and vector containment (contains): whenrelax_methodis a vector, it is unclear whether a condition refers to the whole vector, its first element, or one of its elements. A scalar parameter removes that ambiguity. The existing availability grammar remains unchanged.Unit Tests and/or Case Tests for my changes
Not run: full trajectory comparisons, LCAO/GPU builds, or the full test suite; validation focused on PW/MPI. The broader MPI
ParaReadfixture requires ELPA.Verification commands and build details
GCC 13.4, PW/MPI, LCAO/ELPA disabled, assertions enabled,
OMP_NUM_THREADS=1; executable versionv3.11.0-beta10.$ABACUS_BUILD_DIRis the build directory;$PARAMETER_YAMLand$INPUT_REFERENCEare generated scratch files. CLI input checks used copies oftests/01_PW/060_PW_RE_MG.Quality scoring: 15/17 files passed; both existing low scores are unchanged from the base.
What's changed?
relax_methodas onestd::string. Accept exactly one ofcg(default),cg_trad,bfgs,bfgs_trad,cg_bfgs,lbfgs, orsd; reject empty, unknown, and multi-token values without compatibility aliases.Input_Itemtype/default/help and dependent availability expressions. Invalid empty, unknown, and multi-token values now share one diagnostic that lists every accepted scalar choice. Add assertions that scalar membership is valid and vector containment is rejected forrelax_method.The diff contains 40 files: 9 implementation files, 10 unit-test/fixture files, 2 documentation files, and 19 example/regression INPUT files. Each example/regression INPUT change replaces one relaxation selection.
Governance Notes
relax_methodis now scalar throughout.Input_Itemmetadata is the source of truth; generated YAML and Markdown references were checked but are not committed.