Skip to content

Translate tests#505

Merged
jan-janssen merged 9 commits intomainfrom
translate_tests
Jul 23, 2025
Merged

Translate tests#505
jan-janssen merged 9 commits intomainfrom
translate_tests

Conversation

@jan-janssen
Copy link
Copy Markdown
Member

@jan-janssen jan-janssen commented Jul 23, 2025

Summary by CodeRabbit

  • Refactor
    • Updated tests for elastic properties, energy-volume curve, and phonon calculations to use individual function calls instead of workflow classes, enhancing clarity and maintainability.
    • Introduced new phonon-related functions to obtain band structures and dynamical matrices.
    • Improved functional interfaces for generating tasks, analyzing results, and extracting thermal and phonon properties without altering existing test validations.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jul 23, 2025

Walkthrough

The test files for elastic property and energy-volume curve calculations were refactored to replace workflow class usage with direct functional API calls. Workflow objects were removed in favor of explicitly calling task generation, analysis, and property calculation functions. Additionally, the return order of analyse_results_for_elastic_matrix was swapped and corresponding unpacking adjusted. Phonon and quasi-harmonic approximation tests were similarly refactored to use functional APIs instead of workflow classes. The test logic and assertions remain unchanged.

Changes

File(s) Change Summary
tests/test_elastic_ase_emt.py, tests/test_ase_interface/test_elastic_ase_gpaw.py Replaced ElasticMatrixWorkflow class usage with calls to get_tasks_for_elastic_matrix and analyse_results_for_elastic_matrix functions.
atomistics/workflows/elastic/helper.py, atomistics/workflows/elastic/workflow.py Changed return order of analyse_results_for_elastic_matrix and updated unpacking accordingly.
tests/test_evcurve_ase_*.py (multiple files), tests/test_evcurve_ase_mace.py, tests/test_evcurve_ase_orb.py, tests/test_evcurve_ase_qe.py, tests/test_evcurve_ase_siesta.py, tests/test_evcurve_ase_grace.py, tests/test_evcurve_ase_matgl.py, tests/test_evcurve_ase_emt.py, tests/test_evcurve_ase_lammps.py, tests/test_evcurve_lammpsfile.py, tests/test_evcurve_qe.py, tests/test_evcurve_sphinxdft.py Replaced EnergyVolumeCurveWorkflow class usage with calls to get_tasks_for_energy_volume_curve, analyse_results_for_energy_volume_curve, and get_thermal_properties_for_energy_volume_curve functions; moved fit parameters from workflow instantiation to analysis function calls.
tests/test_elastic_lammpslib_functional.py Adjusted unpacking order of analyse_results_for_elastic_matrix return values to match updated function signature.
atomistics/workflows/init.py Added imports and exports of get_band_structure and get_dynamical_matrix in phonopy workflows.
atomistics/workflows/phonons/helper.py Added new function get_dynamical_matrix to extract the dynamical matrix from a Phonopy object.
tests/test_ase_interface/test_phonons_ase_gpaw.py, tests/test_ase_interface/test_phonons_ase_mace.py, tests/test_ase_interface/test_phonons_ase_matgl.py, tests/test_hessian_lammpslib.py, tests/test_lammpslib_md_phonons.py, tests/test_phonons_ase_emt.py, tests/test_phonons_lammpsfile.py Refactored phonon tests to replace PhonopyWorkflow class with functional API calls including get_tasks_for_harmonic_approximation, analyse_results_for_harmonic_approximation, get_hesse_matrix, get_dynamical_matrix, and get_band_structure.
tests/test_ase_interface/test_quasiharmonic_ase_mace.py, tests/test_ase_interface/test_quasiharmonic_ase_matgl.py, tests/test_quasiharmonic_ase_emt.py Refactored quasi-harmonic approximation tests to replace QuasiHarmonicWorkflow class with functional API calls including get_tasks_for_quasi_harmonic_approximation, analyse_results_for_quasi_harmonic_approximation, and get_thermal_properties_for_quasi_harmonic_approximation.

Sequence Diagram(s)

sequenceDiagram
    participant Test
    participant WorkflowFunctions
    participant Calculator

    Test->>WorkflowFunctions: get_tasks_for_elastic_matrix / get_tasks_for_energy_volume_curve
    WorkflowFunctions-->>Test: task_dict (+ sym_dict)
    Test->>Calculator: Evaluate task_dict with ASE/EMT/Abinit/etc.
    Calculator-->>Test: output_dict
    Test->>WorkflowFunctions: analyse_results_for_elastic_matrix / analyse_results_for_energy_volume_curve (output_dict, sym_dict/fit params)
    WorkflowFunctions-->>Test: analysis_results
    Test->>WorkflowFunctions: get_thermal_properties_for_energy_volume_curve (if applicable)
    WorkflowFunctions-->>Test: thermal_properties
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~20 minutes

Possibly related PRs

  • More functional approach #503: Introduced the functional APIs get_tasks_for_elastic_matrix and analyse_results_for_elastic_matrix that replace the workflow class usage, directly related to this PR's refactor.
  • Reuse the same output patterns #504: Modified the return order and unpacking of analyse_results_for_elastic_matrix, matching changes applied in this PR.
  • Implement optimize volume #431: Refactored workflows and tests to modular function-based interfaces, related to the overall theme of workflow refactoring in this PR.

Poem

Hopping through the code with glee,
I swapped out workflows, set them free!
Now functions dance in tidy rows,
Elastic, energy—each one knows
Their tasks and fits, so clear and neat—
A rabbit’s job is now complete! 🐇


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ffde2d8 and 37b5091.

📒 Files selected for processing (4)
  • tests/test_ase_interface/test_quasiharmonic_ase_mace.py (3 hunks)
  • tests/test_ase_interface/test_quasiharmonic_ase_matgl.py (3 hunks)
  • tests/test_evcurve_lammpsfile.py (2 hunks)
  • tests/test_hessian_lammpslib.py (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • tests/test_hessian_lammpslib.py
  • tests/test_evcurve_lammpsfile.py
  • tests/test_ase_interface/test_quasiharmonic_ase_matgl.py
  • tests/test_ase_interface/test_quasiharmonic_ase_mace.py
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
tests/test_evcurve_ase_emt.py (1)

31-34: Minor: Extra space in parameter

There's an extra space in task_dict , on line 32.

-            task_dict=task_dict ,
+            task_dict=task_dict,
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26e3eac and 00fd97c.

📒 Files selected for processing (2)
  • tests/test_elastic_ase_emt.py (2 hunks)
  • tests/test_evcurve_ase_emt.py (2 hunks)
🔇 Additional comments (7)
tests/test_elastic_ase_emt.py (3)

7-7: LGTM: Clean functional API imports

The import statement correctly brings in the new functional APIs that replace the workflow class approach.


20-25: LGTM: Explicit task generation with clear return values

The functional approach makes the data flow explicit by returning both task_dict and sym_dict from get_tasks_for_elastic_matrix. This is cleaner than the previous workflow object approach as it eliminates hidden state.


27-31: LGTM: Explicit parameter passing for analysis

The analysis function now takes explicit parameters (output_dict, sym_dict, fit_order) making the dependencies clear and improving testability. The functional approach eliminates the need to maintain state in workflow objects.

tests/test_evcurve_ase_emt.py (4)

8-10: LGTM: Complete functional API imports

The import statement correctly includes all three functional APIs needed to replace the energy-volume curve workflow class.


25-30: LGTM: Clear task generation with explicit parameters

The functional approach makes all parameters explicit (structure, num_points, vol_range, axes) which improves code clarity and maintainability compared to the workflow class approach.


35-40: LGTM: Explicit analysis parameters

The functional approach makes all fitting parameters explicit (output_dict, task_dict, fit_type, fit_order) which improves code readability and makes the analysis configuration clear.


41-46: LGTM: Thermal properties calculation with explicit dependencies

The functional approach requires explicit passing of masses from the original structure, making the dependencies clear. The explicit parameter specification (temperatures, output_keys) improves code maintainability.

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.31%. Comparing base (26e3eac) to head (37b5091).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #505      +/-   ##
==========================================
+ Coverage   85.29%   85.31%   +0.01%     
==========================================
  Files          43       43              
  Lines        2584     2587       +3     
==========================================
+ Hits         2204     2207       +3     
  Misses        380      380              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🔭 Outside diff range comments (1)
tests/test_ase_interface/test_phonons_ase_mace.py (1)

64-67: Fix incomplete refactoring - workflow object no longer exists.

These lines still reference a workflow object that was removed during the refactoring. They should use the functional API calls instead.

-        hessmat_shape = workflow.get_hesse_matrix().shape
+        hessmat_shape = get_hesse_matrix(force_constants=phonopy_obj.force_constants).shape
         self.assertEqual(hessmat_shape[0], 324)
         self.assertEqual(hessmat_shape[1], 324)
-        band_dict = workflow.get_band_structure()
+        band_dict = get_band_structure(phonopy=phonopy_obj, npoints=101)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bb7383c and 36ee54e.

📒 Files selected for processing (5)
  • atomistics/workflows/__init__.py (2 hunks)
  • atomistics/workflows/phonons/helper.py (1 hunks)
  • tests/test_ase_interface/test_phonons_ase_gpaw.py (2 hunks)
  • tests/test_ase_interface/test_phonons_ase_mace.py (2 hunks)
  • tests/test_ase_interface/test_phonons_ase_matgl.py (2 hunks)
🧬 Code Graph Analysis (3)
tests/test_ase_interface/test_phonons_ase_matgl.py (3)
atomistics/workflows/structure_optimization.py (1)
  • optimize_positions_and_volume (4-15)
atomistics/workflows/phonons/helper.py (6)
  • get_band_structure (481-526)
  • get_dynamical_matrix (591-610)
  • get_hesse_matrix (421-443)
  • get_tasks_for_harmonic_approximation (263-308)
  • analyse_results_for_harmonic_approximation (311-354)
  • force_constants (156-165)
atomistics/calculators/ase.py (1)
  • evaluate_with_ase (147-205)
atomistics/workflows/phonons/helper.py (1)
atomistics/workflows/phonons/workflow.py (1)
  • get_dynamical_matrix (152-170)
atomistics/workflows/__init__.py (2)
atomistics/workflows/phonons/helper.py (2)
  • get_band_structure (481-526)
  • get_dynamical_matrix (591-610)
atomistics/workflows/phonons/workflow.py (2)
  • get_band_structure (209-231)
  • get_dynamical_matrix (152-170)
🧰 Additional context used
🧬 Code Graph Analysis (3)
tests/test_ase_interface/test_phonons_ase_matgl.py (3)
atomistics/workflows/structure_optimization.py (1)
  • optimize_positions_and_volume (4-15)
atomistics/workflows/phonons/helper.py (6)
  • get_band_structure (481-526)
  • get_dynamical_matrix (591-610)
  • get_hesse_matrix (421-443)
  • get_tasks_for_harmonic_approximation (263-308)
  • analyse_results_for_harmonic_approximation (311-354)
  • force_constants (156-165)
atomistics/calculators/ase.py (1)
  • evaluate_with_ase (147-205)
atomistics/workflows/phonons/helper.py (1)
atomistics/workflows/phonons/workflow.py (1)
  • get_dynamical_matrix (152-170)
atomistics/workflows/__init__.py (2)
atomistics/workflows/phonons/helper.py (2)
  • get_band_structure (481-526)
  • get_dynamical_matrix (591-610)
atomistics/workflows/phonons/workflow.py (2)
  • get_band_structure (209-231)
  • get_dynamical_matrix (152-170)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: unittest_sphinxdft
  • GitHub Check: unittest_siesta
  • GitHub Check: unittest_matrix (windows-latest, 3.12)
  • GitHub Check: unittest_matgl
  • GitHub Check: unittest_gpaw
  • GitHub Check: unittest_matrix (macos-latest, 3.12)
  • GitHub Check: unittest_matrix (ubuntu-latest, 3.11)
  • GitHub Check: unittest_matrix (ubuntu-latest, 3.12)
  • GitHub Check: unittest_qe
  • GitHub Check: unittest_old
  • GitHub Check: notebooks
  • GitHub Check: coverage
🔇 Additional comments (16)
atomistics/workflows/__init__.py (2)

44-45: LGTM! Proper exposure of new functional API components.

The addition of get_band_structure and get_dynamical_matrix to the phonopy workflows list correctly exposes these functions as part of the functional API, supporting the transition away from workflow classes.


59-60: LGTM! Consistent import of new functional API components.

The imports correctly match the functions added to the export list, maintaining consistency in the module's API.

atomistics/workflows/phonons/helper.py (1)

591-610: LGTM! Correct implementation of standalone dynamical matrix function.

The function correctly implements the same logic as the workflow method version (from atomistics/workflows/phonons/workflow.py lines 151-169), properly extracting the functionality into a standalone function that supports the functional API refactor.

tests/test_ase_interface/test_phonons_ase_matgl.py (5)

7-7: LGTM! Proper import of functional API components.

The import statement correctly includes all the necessary functions for the functional API refactor.


33-40: LGTM! Correct transition to functional task generation.

The call to get_tasks_for_harmonic_approximation properly replaces the workflow instantiation and returns both the task dictionary and phonopy object as expected.


45-49: LGTM! Proper functional analysis call.

The analyse_results_for_harmonic_approximation call correctly uses the phonopy object and includes the dos_mesh parameter, properly replacing the workflow method approach.


62-62: LGTM! Correct functional call for Hesse matrix.

The get_hesse_matrix call properly uses the force constants from the phonopy object, correctly replacing the workflow method approach.


65-65: LGTM! Correct functional call for band structure.

The get_band_structure call properly includes the phonopy parameter and maintains the same functionality as the workflow method.

tests/test_ase_interface/test_phonons_ase_gpaw.py (5)

6-6: LGTM! Proper import of functional API components.

The import statement correctly includes all necessary functions for the functional API transition.


21-28: LGTM! Correct functional task generation.

The get_tasks_for_harmonic_approximation call properly replaces workflow instantiation and returns the expected task dictionary and phonopy object.


33-38: LGTM! Proper functional analysis implementation.

The analyse_results_for_harmonic_approximation call correctly uses the phonopy object and includes all necessary parameters, properly replacing the workflow approach.


48-48: LGTM! Correct dynamical matrix function call.

The get_dynamical_matrix call properly includes the required phonopy parameter, correctly implementing the functional API.


54-54: LGTM! Correct band structure function call.

The get_band_structure call properly includes the phonopy parameter and maintains the same functionality as the workflow method.

tests/test_ase_interface/test_phonons_ase_mace.py (3)

7-8: LGTM! Proper import of functional API components.

The import statement correctly includes all necessary functions for the functional API transition.


35-42: LGTM! Correct functional task generation.

The get_tasks_for_harmonic_approximation call properly replaces workflow instantiation and returns the expected task dictionary and phonopy object.


47-51: LGTM! Proper functional analysis implementation.

The analyse_results_for_harmonic_approximation call correctly uses the phonopy object and includes necessary parameters, properly replacing the workflow approach.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🔭 Outside diff range comments (2)
tests/test_evcurve_lammpsfile.py (1)

50-50: Missing import for optimize_positions_and_volume function.

The function optimize_positions_and_volume is called but not imported. This will cause a NameError at runtime.

Add the missing import:

 from atomistics.workflows import (
     analyse_results_for_energy_volume_curve,
     get_tasks_for_energy_volume_curve,
     get_thermal_properties_for_energy_volume_curve,
+    optimize_positions_and_volume,
 )
tests/test_hessian_lammpslib.py (1)

46-54: Fix incorrect parameter placement.

The dos_mesh=20 parameter on line 51 is being passed to get_tasks_for_harmonic_approximation, but this function doesn't accept this parameter. It should be moved to the analyse_results_for_harmonic_approximation call.

 task_dict, phonopy_obj = get_tasks_for_harmonic_approximation(
     structure=result_dict["structure_with_optimized_positions_and_volume"],
     interaction_range=10,
     factor=VaspToTHz,
     displacement=0.01,
-    dos_mesh=20,
     primitive_matrix=None,
     number_of_snapshots=None,
 )
🧹 Nitpick comments (2)
tests/test_phonons_ase_emt.py (1)

30-33: Consider adding explicit dos_mesh parameter for consistency.

The dos_mesh parameter is missing from this call. While it has a default value of 20, other test files in this refactor explicitly pass dos_mesh=20 for consistency.

 phonopy_dict = analyse_results_for_harmonic_approximation(
     phonopy=phonopy_obj,
     output_dict=result_dict,
+    dos_mesh=20,
 )
tests/test_lammpslib_md_phonons.py (1)

44-47: Consider adding explicit dos_mesh parameter for consistency.

While the dos_mesh parameter has a default value of 20, adding it explicitly would be consistent with other test files in this refactor.

 _ = analyse_results_for_harmonic_approximation(
     phonopy=phonopy_obj,
     output_dict=result_dict,
+    dos_mesh=20,
 )
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fd017b2 and ffde2d8.

📒 Files selected for processing (11)
  • tests/test_ase_interface/test_quasiharmonic_ase_mace.py (4 hunks)
  • tests/test_ase_interface/test_quasiharmonic_ase_matgl.py (4 hunks)
  • tests/test_evcurve_ase_lammps.py (3 hunks)
  • tests/test_evcurve_lammpsfile.py (2 hunks)
  • tests/test_evcurve_qe.py (3 hunks)
  • tests/test_evcurve_sphinxdft.py (3 hunks)
  • tests/test_hessian_lammpslib.py (4 hunks)
  • tests/test_lammpslib_md_phonons.py (2 hunks)
  • tests/test_phonons_ase_emt.py (2 hunks)
  • tests/test_phonons_lammpsfile.py (4 hunks)
  • tests/test_quasiharmonic_ase_emt.py (3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
tests/test_evcurve_qe.py (1)
atomistics/workflows/evcurve/helper.py (2)
  • analyse_results_for_energy_volume_curve (239-274)
  • get_tasks_for_energy_volume_curve (204-236)
tests/test_evcurve_sphinxdft.py (3)
atomistics/workflows/evcurve/helper.py (2)
  • analyse_results_for_energy_volume_curve (239-274)
  • get_tasks_for_energy_volume_curve (204-236)
atomistics/workflows/evcurve/debye.py (1)
  • get_thermal_properties_for_energy_volume_curve (370-410)
atomistics/workflows/structure_optimization.py (1)
  • optimize_volume (18-29)
tests/test_phonons_lammpsfile.py (5)
atomistics/workflows/phonons/helper.py (4)
  • analyse_results_for_harmonic_approximation (311-354)
  • get_tasks_for_harmonic_approximation (263-308)
  • get_thermal_properties_for_harmonic_approximation (357-401)
  • force_constants (156-165)
atomistics/workflows/structure_optimization.py (1)
  • optimize_positions_and_volume (4-15)
atomistics/calculators/lammps/filecalculator.py (1)
  • evaluate_with_lammpsfile (213-254)
tests/test_evcurve_lammpsfile.py (1)
  • evaluate_lammps (23-29)
tests/test_optimize_positions_lammpsfile.py (1)
  • evaluate_lammps (20-26)
tests/test_hessian_lammpslib.py (1)
atomistics/workflows/phonons/helper.py (3)
  • get_tasks_for_harmonic_approximation (263-308)
  • analyse_results_for_harmonic_approximation (311-354)
  • force_constants (156-165)
tests/test_evcurve_ase_lammps.py (2)
atomistics/workflows/evcurve/helper.py (2)
  • analyse_results_for_energy_volume_curve (239-274)
  • get_tasks_for_energy_volume_curve (204-236)
atomistics/workflows/evcurve/debye.py (1)
  • get_thermal_properties_for_energy_volume_curve (370-410)
tests/test_ase_interface/test_quasiharmonic_ase_mace.py (2)
atomistics/workflows/quasiharmonic.py (3)
  • get_tasks_for_quasi_harmonic_approximation (366-433)
  • get_thermal_properties_for_quasi_harmonic_approximation (51-166)
  • analyse_results_for_quasi_harmonic_approximation (436-469)
atomistics/workflows/structure_optimization.py (1)
  • optimize_positions_and_volume (4-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: unittest_matrix (macos-latest, 3.12)
  • GitHub Check: unittest_old
  • GitHub Check: unittest_gpaw
  • GitHub Check: unittest_matrix (windows-latest, 3.12)
  • GitHub Check: notebooks
  • GitHub Check: unittest_matrix (ubuntu-latest, 3.12)
  • GitHub Check: unittest_matrix (ubuntu-latest, 3.11)
  • GitHub Check: unittest_siesta
  • GitHub Check: unittest_matgl
  • GitHub Check: unittest_qe
  • GitHub Check: unittest_sphinxdft
  • GitHub Check: coverage
🔇 Additional comments (46)
tests/test_evcurve_qe.py (3)

6-9: Import refactoring looks correct.

The imports have been properly updated to use the new functional API instead of the workflow class.


40-45: Task generation correctly uses functional API.

The get_tasks_for_energy_volume_curve function is properly called with the correct parameters. The fitting parameters are appropriately removed from this step.


55-60: Result analysis correctly implements functional approach.

The analyse_results_for_energy_volume_curve function is called with the proper parameters, including the fitting parameters that were moved from the task generation step.

tests/test_evcurve_sphinxdft.py (3)

9-14: Import refactoring is comprehensive.

The imports correctly include all the new functional API functions. Note that get_thermal_properties_for_energy_volume_curve is imported but not used in this test.


51-56: Task generation follows the new functional pattern.

The function call correctly generates the task dictionary with appropriate parameters.


66-71: Result analysis properly implements functional API.

The analysis function is called with correct parameters, maintaining the same fitting behavior as the original workflow.

tests/test_evcurve_ase_lammps.py (4)

8-10: Import statements correctly updated for functional API.

The imports properly include all three core functions needed for the energy-volume curve workflow.


41-46: Task generation correctly implemented.

The get_tasks_for_energy_volume_curve function is properly called with the optimized structure and correct parameters.


55-60: Result analysis follows functional pattern correctly.

The analysis function properly receives the output and task dictionaries along with the fitting parameters.


61-66: Thermal properties calculation correctly implemented.

The function properly receives the fit dictionary, atomic masses, and other required parameters. The masses are correctly obtained from the structure.

tests/test_evcurve_lammpsfile.py (4)

9-11: Import statements updated for functional API.

The imports correctly include the three main functions for the energy-volume curve workflow.


57-62: Task generation correctly uses functional API.

The function call properly generates the task dictionary with the correct parameters.


69-74: Result analysis correctly implemented.

The analysis function is properly called with the required parameters and fitting options.


75-80: Thermal properties calculation correctly implemented.

The function properly receives all required parameters including the atomic masses and temperature specifications.

tests/test_quasiharmonic_ase_emt.py (6)

8-13: LGTM! Import refactoring is consistent.

The import statements correctly reflect the transition from workflow classes to functional APIs.


26-34: LGTM! Task generation follows the new functional API pattern.

The get_tasks_for_quasi_harmonic_approximation call correctly unpacks to task_dict and qh_dict, which aligns with the function signature in the relevant code snippets.


36-39: LGTM! Result analysis correctly unpacks return values.

The analyse_results_for_quasi_harmonic_approximation call properly unpacks to eng_internal_dict and phonopy_collect_dict, matching the function signature.


40-50: LGTM! First thermal properties call uses correct parameters.

The function call includes all necessary parameters and follows the functional API pattern correctly.


51-60: LGTM! Second thermal properties call with specific temperature range.

The call correctly specifies custom temperatures and output keys with quantum mechanical approach.


65-74: LGTM! Third thermal properties call with classical approach.

The call correctly uses the same parameters but with quantum_mechanical=False for comparison testing.

tests/test_ase_interface/test_quasiharmonic_ase_matgl.py (6)

7-12: LGTM! Import refactoring matches the pattern.

The import statements are consistent with the functional API transition.


38-48: LGTM! Task generation with additional parameters.

The call includes dos_mesh=20 and primitive_matrix=None parameters, which are appropriate for the matgl test case.


53-56: LGTM! Analysis call follows the standard pattern.

The function call and return value unpacking are consistent with the other test files.


57-67: LGTM! Thermal properties call with adjusted temperature range.

The call uses t_max=501 which is appropriate for the matgl test case.


68-77: LGTM! Quantum mechanical thermal properties with [100, 500] temperature range.

The parameters are consistent with the test requirements.


82-91: LGTM! Classical thermal properties calculation.

The call correctly uses the same temperature range with quantum_mechanical=False.

tests/test_ase_interface/test_quasiharmonic_ase_mace.py (6)

7-12: LGTM! Import statements are consistent.

The functional API imports match the refactoring pattern across all test files.


39-49: LGTM! Task generation with comprehensive parameters.

The call includes all necessary parameters including dos_mesh=20 and primitive_matrix=None.


54-57: LGTM! Analysis call follows the established pattern.

The function call and return value handling are consistent.


58-68: LGTM! First thermal properties call with correct parameters.

The function call includes all required parameters with proper syntax.


69-78: LGTM! Quantum mechanical thermal properties calculation.

The call correctly specifies custom temperatures and output keys.


83-92: LGTM! Classical thermal properties calculation.

The final call correctly uses the classical approach for comparison.

tests/test_phonons_ase_emt.py (3)

8-8: LGTM! Import statement correctly updated for functional API.

The import of the new phonon helper functions aligns with the workflow refactor from class-based to functional approach.


21-28: LGTM! Task generation correctly updated to functional API.

The call to get_tasks_for_harmonic_approximation properly returns the tuple (task_dict, phonopy_obj) and all parameters are correctly passed.


35-35: LGTM! Matrix extraction functions correctly implemented.

All calls to get_hesse_matrix, get_dynamical_matrix, and get_band_structure use the correct parameters and are consistent with the new functional API.

Also applies to: 43-43, 46-46, 49-52

tests/test_hessian_lammpslib.py (2)

11-13: LGTM! Import statements correctly updated for functional API.

The new phonon helper function imports align with the workflow refactor pattern.


76-76: LGTM! Hessian matrix extraction correctly implemented.

The call to get_hesse_matrix with force_constants parameter is correct.

tests/test_phonons_lammpsfile.py (6)

9-17: LGTM! Comprehensive import statements for functional API.

All necessary phonon helper functions are correctly imported for the refactored test.


62-69: LGTM! Task generation correctly implemented.

The call to get_tasks_for_harmonic_approximation has the correct parameters and structure.


76-80: LGTM! Analysis function correctly implemented.

The call to analyse_results_for_harmonic_approximation has the correct parameters including the properly placed dos_mesh=20.


82-82: LGTM! Matrix extraction functions correctly implemented.

The calls to get_hesse_matrix and get_dynamical_matrix use the correct parameters.

Also applies to: 146-146, 149-149


90-100: LGTM! Thermal properties functions correctly implemented.

Both calls to get_thermal_properties_for_harmonic_approximation have the correct parameters and structure.

Also applies to: 127-137


152-152: LGTM! Band structure function correctly implemented.

The call to get_band_structure uses the correct parameters.

tests/test_lammpslib_md_phonons.py (3)

7-7: LGTM! Import statement correctly updated for functional API.

The import of the phonon helper functions aligns with the workflow refactor.


32-39: LGTM! Task generation correctly implemented.

The call to get_tasks_for_harmonic_approximation has the correct parameters including the primitive matrix.


51-54: LGTM! Direct phonopy object usage correctly implemented.

The direct access to phonopy object attributes (force_constants, unitcell, primitive_matrix, supercell_matrix) is correct for the functional API approach.

Comment on lines +59 to +62
_ = analyse_results_for_harmonic_approximation(
phonopy=phonopy_obj,
output_dict=result_dict,
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add missing dos_mesh parameter to analysis function.

The dos_mesh parameter should be added here since it was incorrectly placed in the task generation call.

 _ = analyse_results_for_harmonic_approximation(
     phonopy=phonopy_obj,
     output_dict=result_dict,
+    dos_mesh=20,
 )
📝 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.

Suggested change
_ = analyse_results_for_harmonic_approximation(
phonopy=phonopy_obj,
output_dict=result_dict,
)
_ = analyse_results_for_harmonic_approximation(
phonopy=phonopy_obj,
output_dict=result_dict,
dos_mesh=20,
)
🤖 Prompt for AI Agents
In tests/test_hessian_lammpslib.py around lines 59 to 62, the call to
analyse_results_for_harmonic_approximation is missing the dos_mesh parameter.
Add the dos_mesh argument to this function call with the appropriate value to
ensure the parameter is correctly passed during analysis.

@jan-janssen jan-janssen merged commit e2b80a1 into main Jul 23, 2025
28 checks passed
@jan-janssen jan-janssen deleted the translate_tests branch July 23, 2025 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants