Skip to content

Reduce trig calls via some identities - #380

Merged
scal444 merged 4 commits into
NVIDIA-BioNeMo:mainfrom
scal444:perf-campaign-mmff-chebyshev_torsion
Oct 28, 2026
Merged

scal444 merged 4 commits into
NVIDIA-BioNeMo:mainfrom
scal444:perf-campaign-mmff-chebyshev_torsion

Conversation

@scal444

@scal444 scal444 commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

The torsion energy took acos of cos(phi) and then cos(2 phi) and cos(3 phi).
Compute both from cos(phi) with cos 2x = 2c^2 - 1 and cos 3x = (4c^2 - 3) c
instead, which removes one acos and two cos calls per torsion. The gradient
already used the cos(phi) form. Shared by the batched and per-molecule MMFF
paths.
@greptile-apps

greptile-apps Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The changes since the previous review appear safe to merge.

Summary

Replaces repeated trigonometric calls with equivalent formulas in stereochemistry checks, torsion energy, and eigenvalue calculations.

  • ETKDG checks double-bond stereo from the plane-normal dot-product sign.
  • MMFF torsion energy uses cosine identities instead of rebuilding the angle.
  • The 3×3 eigenvalue calculation gets shifted roots from one sine/cosine pair.

Reviews (2) · Last reviewed commit: "Clarify trigonometric identities and par..." · Reviewed by Greptile

@scal444
scal444 requested a review from evasnow1992 October 28, 2026 14:16

@evasnow1992 evasnow1992 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the consolidation. Changes look good to me!

@scal444
scal444 merged commit 763b923 into NVIDIA-BioNeMo:main Oct 28, 2026
14 checks passed
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