Skip to content
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

Feature 103 #120

Merged
merged 9 commits into from
Oct 30, 2023
Merged

Feature 103 #120

merged 9 commits into from
Oct 30, 2023

Conversation

chryswoods
Copy link
Contributor

This pull request fixes issue #103

  • I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): [y]
  • I confirm that I have added a changelog entry to the changelog (we will add a link to this PR as part of the review): [y]
  • I confirm that I have permission to release this code under the GPL3 license: [y]

Suggested reviewers:

@lohedges

Any additional context of information?

This adds in rotation of Velocities when rotations are performed via a cursor. This builds on the other two PRs so will accept all three once accepted. You can toggle rotation of velocities using the rotate_velocities flag. This defaults to None, which will be True if the user hasn't specified this in the property map (a rotate_velocities flag is set which is passed down to the rotation functions in moverbase.cpp)

the dynamics code.

Added tests and source documentation.
of the 3 angles. The angles are converted from degrees to radians
when held internally. But they should exactly convert back to the
degrees value via `ang.to(degrees)` as I think division/multiplication
by the same number is reversible.

[ci skip]
[ci skip]
part of the unit test that checks that velocity rotation is working.
@chryswoods chryswoods linked an issue Oct 25, 2023 that may be closed by this pull request
@chryswoods chryswoods temporarily deployed to sire-build October 25, 2023 18:35 — with GitHub Actions Inactive
@chryswoods chryswoods temporarily deployed to sire-build October 25, 2023 18:35 — with GitHub Actions Inactive
@chryswoods chryswoods temporarily deployed to sire-build October 25, 2023 18:35 — with GitHub Actions Inactive
@chryswoods chryswoods temporarily deployed to sire-build October 25, 2023 18:35 — with GitHub Actions Inactive
@chryswoods chryswoods temporarily deployed to sire-build October 25, 2023 18:35 — with GitHub Actions Inactive
@lohedges
Copy link
Contributor

I guess I'd need to call this twice for perturbable molecules, i.e. to rotate both end states. This isn't a problem, since it's what I need to do now anyway. (Just remapping the coordinates property as required.)

@chryswoods chryswoods merged commit 3f23cdc into devel Oct 30, 2023
@chryswoods chryswoods deleted the feature_103 branch October 30, 2023 18:17
lohedges added a commit to OpenBioSim/biosimspace that referenced this pull request Nov 13, 2023
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] cursor.rotate should rotate all vector properties
2 participants