Skip to content

Conversation

@jan-janssen
Copy link
Member

@jan-janssen jan-janssen commented Nov 4, 2025

Summary by CodeRabbit

  • New Features
    • Added cell matrix tracking during thermal expansion calculations. Users can now retrieve the unit cell structures alongside temperature and volume data in thermal expansion analysis outputs across ASE and LAMMPS calculators.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

Walkthrough

Cell data tracking has been integrated into the thermal expansion calculation pipeline. Calculators collect cell matrices during molecular dynamics runs, pass them through the thermal expansion output system via updated function signatures, and expose the data through new public methods.

Changes

Cohort / File(s) Summary
Calculator cell collection
atomistics/calculators/ase.py, atomistics/calculators/lammps/helpers.py
Added cell_md_lst to accumulate cell matrices during each temperature iteration in thermal expansion loops; appended final cell from each run to list and passed collected cells to get_thermal_expansion_output.
Thermal expansion core logic
atomistics/shared/thermal_expansion.py
Updated ThermalExpansionProperties.__init__() to accept cell_lst parameter; added new public method cells() returning cell array; updated get_thermal_expansion_output() signature to accept and forward cell_lst to properties constructor.
Thermal expansion output mapping
atomistics/shared/output.py
Added cells: callable field to OutputThermalExpansion dataclass, enabling cell data exposure in output structure.
Phonons thermal properties
atomistics/workflows/phonons/helper.py
Added new public method cells() to PhonopyThermalProperties returning cell array repeated for each temperature, mirroring existing volumes() interface.

Sequence Diagram

sequenceDiagram
    participant Calc as Calculator<br/>(ASE/LAMMPS)
    participant TEOut as get_thermal_expansion_output()
    participant TEProp as ThermalExpansionProperties
    participant OutObj as OutputThermalExpansion

    loop For each temperature
        Calc->>Calc: Collect cell from MD result
        Calc->>Calc: Append to cell_md_lst
    end

    Calc->>TEOut: Pass cell_lst, temperatures_lst,<br/>volumes_lst
    TEOut->>TEProp: Create with cell_lst,<br/>temperatures_lst, volumes_lst
    TEProp->>TEProp: Store as _cell_lst
    TEOut->>OutObj: Map properties to output<br/>(including cells() method)
    OutObj-->>Calc: Return with cell access

    Note over OutObj: OutputThermalExpansion now<br/>provides cells() callable
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Signature propagation: get_thermal_expansion_output() and ThermalExpansionProperties.__init__() signatures changed; verify all call sites in ASE and LAMMPS calculators correctly pass cell_lst.
  • Cell data extraction: Confirm ASE and LAMMPS correctly extract cells at each temperature step (examine result_dict and lmp_instance.interactive_cells_getter() behaviors).
  • Dataclass integration: Verify OutputThermalExpansion properly maps the new cells field through the output object's public interface.

Poem

🐰 Cells through the pipeline hop,
Each temperature gets a snapshot,
Matrices collected with care,
Thermal expansion's full affair!
New methods bloom, the data flows—
All cells accounted as MD goes. 🔬

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main change: adding cell dimension tracking to thermal expansion calculations across multiple modules.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch thermal_expansion_store_cell

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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
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 (2)
atomistics/shared/thermal_expansion.py (2)

16-20: Docstring missing cell_lst parameter

Please update the constructor docstring so it documents the newly added cell_lst argument; it keeps the API description aligned with the signature.

         """
         Initialize the ThermalExpansionProperties class.
 
         Parameters:
-        temperatures_lst (np.ndarray): Array of temperatures.
-        volumes_lst (np.ndarray): Array of volumes.
+        cell_lst (np.ndarray): Array of cell matrices.
+        temperatures_lst (np.ndarray): Array of temperatures.
+        volumes_lst (np.ndarray): Array of volumes.

62-64: Fix docstring typo for cell_lst

The parameter name in the docstring should match the function signature (cell_lst instead of cells_lst).

-    cells_lst (np.ndarray): Array of cells.
+    cell_lst (np.ndarray): Array of cells.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 72bec60 and 7413629.

📒 Files selected for processing (5)
  • atomistics/calculators/ase.py (2 hunks)
  • atomistics/calculators/lammps/helpers.py (2 hunks)
  • atomistics/shared/output.py (1 hunks)
  • atomistics/shared/thermal_expansion.py (2 hunks)
  • atomistics/workflows/phonons/helper.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
atomistics/shared/output.py (2)
atomistics/shared/thermal_expansion.py (1)
  • cells (24-31)
atomistics/workflows/phonons/helper.py (1)
  • cells (227-237)
atomistics/calculators/ase.py (1)
atomistics/shared/thermal_expansion.py (1)
  • get_thermal_expansion_output (52-75)
atomistics/calculators/lammps/helpers.py (1)
atomistics/shared/thermal_expansion.py (1)
  • get_thermal_expansion_output (52-75)
atomistics/workflows/phonons/helper.py (2)
atomistics/shared/thermal_expansion.py (1)
  • cells (24-31)
atomistics/calculators/ase.py (1)
  • cell (100-107)
atomistics/shared/thermal_expansion.py (1)
atomistics/workflows/phonons/helper.py (1)
  • cells (227-237)
⏰ 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_old
  • GitHub Check: unittest_siesta
  • GitHub Check: notebooks
  • GitHub Check: unittest_qe
  • GitHub Check: unittest_matrix (ubuntu-latest, 3.13)
  • GitHub Check: unittest_matrix (macos-latest, 3.14)
  • GitHub Check: unittest_matrix (ubuntu-latest, 3.12)
  • GitHub Check: unittest_matrix (ubuntu-latest, 3.11)
  • GitHub Check: unittest_matrix (windows-latest, 3.14)
  • GitHub Check: unittest_matrix (ubuntu-latest, 3.14)
  • GitHub Check: unittest_sphinxdft
  • GitHub Check: coverage

@jan-janssen jan-janssen marked this pull request as draft November 4, 2025 09:30
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