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 106 #119

Merged
merged 6 commits into from
Oct 30, 2023
Merged

Feature 106 #119

merged 6 commits into from
Oct 30, 2023

Conversation

chryswoods
Copy link
Contributor

This pull request fixes issue #106

  • 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?

I've skipped CI as you will need to check this with BioSimSpace first to see if this is a workable solution. This returns a list of angle objects, so there is some internal conversion from the raw double values in degrees to double values in radians. Internally in AmberRst7, the data is always stored in raw double values in degrees. The conversion is only happening for the list of angles that we return when calling boxAngles.

As this is a list, it has a different API to a Vector, so it may break existing BioSimSpace code.

[ci skip]

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]
@chryswoods chryswoods linked an issue Oct 25, 2023 that may be closed by this pull request
@lohedges
Copy link
Contributor

Thanks. I'm only using this in the Trajectory code at present. I'll just need to add in conversion from radians to degrees.

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] AmberRst7.box_angles() objects have the wrong units
2 participants