-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed the unit error in documentation of phonon.py #17
Conversation
The unit is incorrect. Signed-off-by: Runze Liu <[email protected]>
matcalc/phonon.py
Outdated
free_energy: list of Helmholtz free energies at corresponding temperatures in eV, | ||
entropy: list of entropies at corresponding temperatures in eV/K, | ||
heat_capacity: list of heat capacities at constant volume at corresponding temperatures in eV/K^2, | ||
free_energy: list of Helmholtz free energies at corresponding temperatures in KJ/mol, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! can you add a code comment with a link to where the units are documented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about just writing the comments and links here? Or it may affect the aesthetics of the source code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feel free to insert the link where you think best
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 8 8
Lines 266 266
=========================================
Hits 266 266 ☔ View full report in Codecov by Sentry. |
Tests are also needed. |
Add code comments with links to show where the units are documented originally. Signed-off-by: Runze Liu <[email protected]>
The units are documented in phonopy. See phonopy.Phonopy.run_thermal_properties() (https://github.com/phonopy/phonopy/blob/develop/phonopy/api_phonopy.py#L2591) |
Delete previous comments that were too long. Signed-off-by: Runze Liu <[email protected]>
The links should be in the code documentation itself. That does not affect the aesthetics of the code. It is more important that the documentation is clear. Otherwise it is hard for people to debug next time. |
Add the comments and links. Signed-off-by: Runze Liu <[email protected]>
Signed-off-by: Runze Liu <[email protected]>
Important lesson professor, and thank you both.
|
The unit is incorrect.
Summary
Major changes:
Todos
If this is work in progress, what else needs to be done?
Checklist
ruff
.mypy
.duecredit
@due.dcite
decorators to reference relevant papers by DOI (example)Tip: Install
pre-commit
hooks to auto-check types and linting before every commit: