#65 Fix garbled string energy unit in write_std_multixyz - #69
Merged
Conversation
pyGSM's internal energies are already in kcal/mol (base_lot.get_energy returns Hartree * KCAL_MOL_PER_AU). write_std_multixyz multiplied them by KJ_MOL_TO_AU, treating a kcal/mol value as kJ/mol and emitting a garbled AU-scaled number that downstream code had to reverse-engineer. Write the energy in its native kcal/mol unit with an explicit "kcal/mol" label so the output is a clean, documented, self-identifying unit. Geometry readers in this repo parse only atom lines, so the label is safe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FZskcb9ogmfHPD1GAw1VDD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
write_std_multixyz(pyGSM/utilities/manage_xyz.py) multiplied each node's energy byunits.KJ_MOL_TO_AUbefore writing it to the XYZ comment line. The energy passed in is already in kcal/mol (level_of_theories/base_lot.py'sget_energyreturnsHartree * KCAL_MOL_PER_AU), so this treated a kcal/mol value as if it were kJ/mol and produced a garbled AU-scaled number — not documented, not any real unit.Fix: write the value as-is, in its native kcal/mol, with an explicit
kcal/mollabel so the format is self-documenting and downstream readers can detect it (py-conformational-sampling#86,ZimmermanGroup/asymmetric-aminoallylation#3).Consequence for existing consumers: any tool that read the old comment-line value directly will see roughly a 4.184x change in energies for files written after this fix (that was the actual bug — the old numbers were off by that factor).
py-conformational-sampling'sanalyze.pyandasymmetric-aminoallylation'sdata.pyboth now detect the format via the unit label so old and new files read back correctly; see the linked PRs/issues.Todos
write_std_multixyz's comment-line format; ran the full existing suiteQuestions
Status
pytest pyGSM/tests/passes (2/2) against this branchCloses #65
Generated by Claude Code