Standarize pressure to kPa - #179
mattwthompson wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #179 +/- ##
=======================================
Coverage 81.40% 81.40%
=======================================
Files 56 56
Lines 5070 5070
=======================================
Hits 4127 4127
Misses 943 943 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The pressure_tolerance default values in store.py (still 0.001) were not updated to the kPa-equivalent 0.1, so the effective default tolerance silently changes and contradicts the updated docstrings.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
This PR standardizes the pressure unit to kilopascals (kPa) throughout tyff, resolving the inconsistency described in issue #177 where pressures were expressed in atm in some places and kPa in others. It updates the SimulationConfig field type, the thermo target (config construction, evaluator import, observable computation), reporter/MM tests, and the pressure-related docstrings in the coordinates store.
Changes:
- Switch pressure unit type/usages from
atmosphere/atmtokilopascal/kPainmm/_config.pyandtargets/thermo.py. - Update pressure docstrings in
coordinates/store.pyto kPa and adjust documented default tolerance to0.1. - Update tests to use kilopascal, including the expected evaluator density-import pressure (
0.999753… → 101.3).
| File | Description |
|---|---|
| tyff/mm/_config.py | Changes SimulationConfig.pressure type to kilopascal. |
| tyff/targets/thermo.py | Converts config construction, evaluator import, and observable computation to kPa; updates docstrings. |
| tyff/coordinates/store.py | Updates pressure/tolerance docstrings to kPa and documents default=0.1 tolerance. |
| tyff/_tests/targets/test_thermo.py | Updates assertions/config to kilopascal and expected import value to 101.3. |
| tyff/_tests/mm/test_reporters.py | Switches reporter/barostat test pressures to kilopascal. |
| tyff/_tests/mm/test_mm.py | Switches simulation test pressure to kilopascal. |
Note: tyff/coordinates/box.py (not in this diff) still describes its pressure fields as "Pressure in atm" (lines 235, 263); those descriptions feed the now-kPa store.py filters and are left inconsistent.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Closes #177