perf: accumulate ANI2xt forward energies in float64 (draft — needs torchani verification)#96
Draft
isayev wants to merge 1 commit into
Draft
perf: accumulate ANI2xt forward energies in float64 (draft — needs torchani verification)#96isayev wants to merge 1 commit into
isayev wants to merge 1 commit into
Conversation
… energies) The energy_shifts buffer is float64 but the forward accumulated atom_energies / self_energies in float32 (coords dtype), immediately downcasting it. Accumulate in float64 so the buffer is used at full precision and absolute energies are clean. Ranking is unchanged: per-atom self-energies depend only on atom counts and cancel exactly in conformer energy differences. The AEV and per-element networks remain float32. NOTE: cannot be verified in this environment (torchani, required to construct ANI2xt, is not installed and no fast test exercises this forward). Gated on the CI [ani] job / a torchani-equipped review before merge -- hence draft.
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
1 participant
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.
Workstream W8 (lowest priority, cosmetic). Draft — see verification caveat.
ANI2xt's
energy_shiftsbuffer is float64 but the forward accumulatedatom_energies/self_energiesin float32, immediately downcasting it. This accumulates them in float64 so the buffer is used at full precision and absolute energies are clean. The AEV and per-element networks stay float32.This does NOT change conformer rankings — per-atom self-energies depend only on atom counts and cancel exactly in energy differences within a molecule (confirmed by the round-3 analysis). It only cleans up absolute energies.
I could not verify this locally:
torchani(required even to constructANI2xt) is not installed in the dev environment, and no fast test exercises this forward. Dtype-promotion and import were checked (f32 networks + f64 self-energies → f64 total; module imports clean), but the actual energy values were not run. Do not merge until the CI[ani]job (or a maintainer with torchani) runs the ANI2xt SPE/thermo tests and confirms energies are unchanged within tolerance. Kept as a draft for that reason. Low value — fine to close if not worth the verification effort.