Repository navigation
refactor(input): declare dfpt mixing parameter domains (set_choices / set_rule) - #8251
Merged
Merged
Conversation
…et_rule) Follow-up to PR deepmodeling#8214 review finding A10: replace the hand-written check_value lambdas with the declarative domain system so the constraints appear in the generated help and rejection routes through the shared input-error path. - dfpt_mix_type: domain.set_choices({rho_plain, rho_kerker, rho_broyden, v_sc_plain, v_sc_broyden}) - dfpt_kerker_a2: domain.set_rule("value>0") - dfpt_broyden_ndim: domain.set_rule("value>=1") No behavior change; DFPT serial 9/9, INPUT domain/reader/help tests all pass. MODULE_IO_input_test_para fails identically on the develop baseline (pre-existing MPI environment issue).
Zanthoxylum
added
Refactor
ABACUS-PR-BOT
left a comment
ABACUS-PR-BOT
left a comment
Collaborator
There was a problem hiding this comment.
Static review at 93179c0: the five choices, numeric bounds and omitted defaults are preserved. One nonblocking note covers the explicit-empty input contract and focused reader/help coverage. No build or test was run.
…add reader tests The scalar choices domain now rejects an explicitly empty dfpt_mix_type before read_value runs, so the legacy get_size()==0 fallback (which silently mapped an empty value onto v_sc_broyden) was unreachable. Replace the hand-written read_value with read_sync_string and record the contract in a comment: omitted line keeps the default, an explicit empty or unknown value is rejected. Add reader tests for the three registrations: all five dfpt_mix_type choices plus empty/unknown/multi-token rejection, dfpt_kerker_a2 with the a^2=0 boundary under value>0, and dfpt_broyden_ndim with the ndim=0 boundary under value>=1. Each rejection also checks that read_value is not reached (previous value survives) and the doc anchor.
…elop into dfpt-input-domain
ABACUS-PR-BOT
left a comment
ABACUS-PR-BOT
left a comment
Collaborator
There was a problem hiding this comment.
No new implementation defect found at 6ef7199. fd271f168 resolves the explicit-empty contract concern and adds the requested rejection cases.
Static source review covered the PR and affected interfaces at develop 75e2a62. The author-reported 3/3 tests and CLI checks were not independently rerun; merged-build verification remains unperformed.
mohanchen
approved these changes
Nov 1, 2026
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.
Summary
Follow-up to PR #8214 review finding A10: declare the DFPT mixing INPUT parameter domains with the declarative
set_choices/set_rulesystem instead of hand-writtencheck_valuelambdas, so the constraints appear in the generated help and rejection routes through the shared input-error path.What's changed?
dfpt_mix_type:domain.set_choices({"rho_plain", "rho_kerker", "rho_broyden", "v_sc_plain", "v_sc_broyden"})— replaces the 5-way string comparison incheck_valuedfpt_kerker_a2:domain.set_rule("value>0")— replaces the positive checkdfpt_broyden_ndim:domain.set_rule("value>=1")— replaces the ≥1 checkNo behavior change; the same invalid values are rejected with the same error path.
Unit Tests and/or Case Tests for my changes
MODULE_IO_input_domain,MODULE_IO_input_domain_reader,MODULE_IO_read_input_serial,MODULE_IO_read_input_tool,MODULE_IO_input_help_test)