fix: qe input blocks not seperated by empty lines #724
Conversation
WalkthroughWalkthroughThe changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SCFModule as SCF Module
participant OutputFile as Quantum Espresso Output File
participant TestSuite as Test Suite
User->>SCFModule: Request data extraction
SCFModule->>OutputFile: Read output data
SCFModule->>SCFModule: Identify blocks using _QE_BLOCK_KEYWORDS
SCFModule->>OutputFile: Extract forces and stress data
alt No stress data found
SCFModule-->>User: Return None
else Stress data found
SCFModule-->>User: Return stress data
end
User->>TestSuite: Run tests
TestSuite->>SCFModule: Validate LabeledSystem with output data
TestSuite-->>User: Return test results
Recent review detailsConfiguration used: CodeRabbit UI Files ignored due to path filters (1)
Files selected for processing (3)
Files skipped from review due to trivial changes (1)
Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
wanghan-iapcm
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- dpdata/qe/scf.py (3 hunks)
- tests/test_qe_pw_scf.py (1 hunks)
Additional context used
Ruff
tests/test_qe_pw_scf.py
194-194: Local variable
b2ais assigned to but never usedRemove assignment to unused variable
b2a(F841)
Additional comments not posted (5)
dpdata/qe/scf.py (4)
14-26: LGTM!The addition of the
_QE_BLOCK_KEYWORDSlist is a good practice for standardizing the parsing of QE output data. It improves code readability and maintainability.
34-39: LGTM!The modification of the
get_blockfunction to utilize the_QE_BLOCK_KEYWORDSlist enhances the robustness of block extraction. It ensures that only relevant lines are included in the returned block, preventing the inclusion of unwanted lines that could disrupt data parsing.
131-132: LGTM!The modification of the
get_forcefunction to raise aRuntimeErrorwhen no force data is found improves error handling. It provides clearer feedback to users, making it easier to identify and address issues related to missing force data in the output file.
145-145: LGTM!The modification of the
get_stressfunction to returnNonewhen no stress data is found clarifies the function's behavior. It allows for better handling of the function's output and makes it easier to differentiate between the absence of stress data and an empty stress tensor.tests/test_qe_pw_scf.py (1)
191-234: LGTM!The new test class
TestNais a valuable addition to the test suite. It thoroughly validates the functionality of thedpdata.LabeledSystemclass when processing a Quantum ESPRESSO output file for a sodium atom configuration.The test method checks various properties of the labeled system, including the number of atoms, atom names, number of frames, atom types, coordinates, cell dimensions, and forces. The assertions confirm that the system correctly identifies three sodium atoms, verifies their names, and checks that the coordinates and cell parameters match expected values. It also ensures that the forces acting on the atoms are zero, indicating a stable configuration.
The use of
np.testing.assert_array_equalandnp.testing.assert_allclosefunctions ensures that the comparisons are precise, allowing for numerical tolerance in floating-point operations.Overall, this addition enhances the test coverage for the
dpdatalibrary by specifically targeting sodium atom configurations and ensures that thedpdata.LabeledSystemclass can correctly process Quantum ESPRESSO output files and extract relevant information.Tools
Ruff
194-194: Local variable
b2ais assigned to but never usedRemove assignment to unused variable
b2a(F841)
tests/test_qe_pw_scf.py
Outdated
| class TestNa(unittest.TestCase): | ||
| def test(self): | ||
| ss = dpdata.LabeledSystem("qe.scf/na.out", fmt="qe/pw/scf") | ||
| b2a = dpdata.unit.LengthConversion("bohr", "angstrom").value() |
There was a problem hiding this comment.
Remove the unused variable b2a.
The local variable b2a is assigned the value of dpdata.unit.LengthConversion("bohr", "angstrom").value() but it is not used anywhere else in the test method.
Apply this diff to remove the unused variable:
- b2a = dpdata.unit.LengthConversion("bohr", "angstrom").value()Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| b2a = dpdata.unit.LengthConversion("bohr", "angstrom").value() |
Tools
Ruff
194-194: Local variable
b2ais assigned to but never usedRemove assignment to unused variable
b2a(F841)
CodSpeed Performance ReportMerging #724 will not alter performanceComparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #724 +/- ##
==========================================
+ Coverage 84.51% 84.53% +0.01%
==========================================
Files 81 81
Lines 7155 7156 +1
==========================================
+ Hits 6047 6049 +2
+ Misses 1108 1107 -1 ☔ View full report in Codecov by Sentry. |
the qe support only parse input file, in which cell blocks are separated by empty lines, like
however, the input file is valid when no empty line exists, like the following
This pr fixes the issue
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests
dpdata.LabeledSystemclass with Quantum Espresso output, enhancing test coverage.