Refactor: Improve stability of KSDFT-generated ML descriptors#7431
Open
sunliang98 wants to merge 2 commits into
Open
Refactor: Improve stability of KSDFT-generated ML descriptors#7431sunliang98 wants to merge 2 commits into
sunliang98 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens correctness/stability around KSDFT-generated ML descriptor output by (1) enforcing valid runtime conditions for of_ml_gene_data and (2) extending the integration test harness to record/compare summary statistics of generated .npy descriptors, along with a new PW test case exercising that path.
Changes:
- Add an input validation check that rejects
of_ml_gene_dataunless running KSDFT + PW on a single MPI rank (NPROC=1). - Extend integration tooling to compute and compare mean absolute values of ML descriptor
.npyoutputs. - Add a new integrated example/test case
01_PW/103_PW_Gene_Descriptorsand include it in the CPU case list.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integrate/tools/collect_npy_means.py | New helper to compute per-.npy mean(abs(values)) and print as key/value pairs for reference comparison. |
| tests/integrate/tools/catch_properties.sh | Appends collected descriptor mean metrics to result.out/result.ref when descriptor output directory exists. |
| tests/integrate/Autotest.sh | Adds a descriptor-specific threshold path and forces single-rank execution when of_ml_gene_data=1 is detected. |
| tests/01_PW/CASES_CPU.txt | Registers the new 103_PW_Gene_Descriptors test case. |
| tests/01_PW/103_PW_Gene_Descriptors/STRU | New structure input for descriptor-generation case. |
| tests/01_PW/103_PW_Gene_Descriptors/KPT | New k-point mesh for the descriptor-generation case. |
| tests/01_PW/103_PW_Gene_Descriptors/INPUT | Enables of_ml_gene_data and related ML-KEDF descriptor parameters. |
| tests/01_PW/103_PW_Gene_Descriptors/result.ref | Adds reference outputs including descriptor mean keys for regression testing. |
| tests/01_PW/103_PW_Gene_Descriptors/README | Documents intent/constraints of the new descriptor-generation case. |
| source/source_io/module_parameter/read_input_item_ofdft.cpp | Adds check_value validation for of_ml_gene_data (KSDFT+PW+single-rank only). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+389
to
+392
| ModuleBase::WARNING_QUIT( | ||
| "ReadInput", | ||
| "of_ml_gene_data is only available for KSDFT with PW basis on a single core (NPROC = 1)"); | ||
| } |
Comment on lines
+385
to
+393
| item.check_value = [](const Input_Item& item, const Parameter& para) { | ||
| if (para.input.of_ml_gene_data | ||
| && (para.input.esolver_type != "ksdft" || para.input.basis_type != "pw" || GlobalV::NPROC != 1)) | ||
| { | ||
| ModuleBase::WARNING_QUIT( | ||
| "ReadInput", | ||
| "of_ml_gene_data is only available for KSDFT with PW basis on a single core (NPROC = 1)"); | ||
| } | ||
| }; |
Comment on lines
290
to
+294
| my_threshold=$(get_threshold $threshold_file "threshold" $threshold) | ||
| my_force_threshold=$(get_threshold $threshold_file "force_threshold" $force_threshold) | ||
| my_stress_threshold=$(get_threshold $threshold_file "stress_threshold" $stress_threshold) | ||
| my_fatal_threshold=$(get_threshold $threshold_file "fatal_threshold" $fatal_threshold) | ||
| check_out result.out $my_threshold $my_force_threshold $my_stress_threshold $my_fatal_threshold | ||
| check_out result.out $my_threshold $my_force_threshold $my_stress_threshold $my_fatal_threshold $descriptor_threshold |
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
Linked Issue
Fix #...
Unit Tests and/or Case Tests for my changes
What's changed?
This PR introduces two main improvements:
Add check for
of_ml_gene_dataA validation step has been added to ensure that
of_ml_gene_datais only used with single-core PW basis set KSDFT calculations. If the current calculation does not meet this requirement, an error will be raised with an appropriate message, preventing invalid or unsupported usage.Add integrated example
01_PW/103_PW_Gene_DescriptorsAny changes of core modules? (ignore if not applicable)