Skip to content

Remove length unit scaling from NORMAL/UNIFORM direction cosines - #345

Merged
drobnyjt merged 1 commit into
lcpp-org:mainfrom
GuySten:fix-direction-scaling
Oct 2, 2026
Merged

drobnyjt merged 1 commit into
lcpp-org:mainfrom
GuySten:fix-direction-scaling

Conversation

@GuySten

@GuySten GuySten commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Thank you for your contribution to rustBCA!

Before submitting this PR, please make sure you have:

  • Opened an issue
  • Referenced the relevant issue number(s) below
  • Provided a description of the changes below
  • Ensured all tests pass and added any necessary tests for new code

Fixes #344

Description

The direction cosines for the NORMAL/UNIFORM distributions were scaled by length_unit.
This PR removes the length_unit scaling from the direction cosines in the UNIFORM/NORMAL distribution cases.

Tests

cargo test passes on my machine. (Ubuntu 24.04.1 LTS (WSL2 on Windows 11, x86_64), rustc 1.98.1).
After this fix the MWE from #344 works correctly.

❯ RustBCA 0D mwe_angstrom.toml
❯ awk -F, 'NR==1{x=$4;y=$5;z=$6} NR==3{dx=$4-x;dy=$5-y;dz=$6-z;n=sqrt(dx*dx+dy*dy+dz*dz);printf "%.4f %.4f %.4f\n",dx/n,dy/n,dz/n; exit}' angstrom_trajectories.output
0.5809 0.8140 0.0000

Generative AI Use

Claude helped find the bug and the fix, and generated the MWE and the awk command.

@GuySten
GuySten marked this pull request as ready for review September 29, 2026 14:16
@GuySten GuySten changed the title Remove length unit scaling from direction cosines Remove length unit scaling from NORMAL/UNIFORM direction cosines Sep 29, 2026
@drobnyjt
drobnyjt self-requested a review October 2, 2026 19:27
@drobnyjt
drobnyjt merged commit a356280 into lcpp-org:main Oct 2, 2026
2 checks passed
@drobnyjt

drobnyjt commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

@GuySten Thank you very much!

@GuySten
GuySten deleted the fix-direction-scaling branch October 2, 2026 19:51
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.

[bug] Direction cosines are scaled by length_unit for the UNIFORM/NORMAL distribution.

2 participants