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.

Sign up for GitHub

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

Jump to bottom

fix dihedral angle computation #7892

Merged
merged 10 commits into from
Dec 11, 2023
Merged

fix dihedral angle computation #7892

merged 10 commits into from
Dec 11, 2023

Conversation

sloriot
Copy link
Member

@sloriot sloriot commented Nov 28, 2023 •
edited
Loading

No description provided.

@sloriot sloriot force-pushed the CGAL-fix_dh_angle_sign branch from b6be89b to 6b570f7 Compare November 28, 2023 17:10
@sloriot sloriot added Batch_1 First Batch of PRs under testing and removed Work in progress labels Nov 28, 2023
@MaelRL MaelRL added the TODO label Nov 29, 2023
@MaelRL MaelRL added this to the 5.5.4 milestone Nov 29, 2023
@sloriot
accomodate handle sign
e3e5bcd 
face orientation was inversed and somehow matched the bug
in the dihedral angle computation function
@sloriot sloriot added Under Testing and removed Batch_1 First Batch of PRs under testing labels Nov 29, 2023
@afabri
Copy link
Member

afabri commented Nov 30, 2023

@sloriot can you check that compare_dihedral_angle() is correct.

@lrineau
Copy link
Member

lrineau commented Nov 30, 2023

@sloriot can you check that compare_dihedral_angle() is correct.

Note that CGAL::compare_dihedral_angle compares *unsigned angles (in [0, pi]) whereas approximate_dihedral_angle computes the signed dihedral angle (in ]-pi, pi]). At least that is documented that way.

@sloriot
Copy link
Member Author

sloriot commented Dec 7, 2023

Successfully tested in CGAL-6.0-Ic-122

@lrineau lrineau self-assigned this Dec 7, 2023
@lrineau lrineau added the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Dec 7, 2023
@MaelRL MaelRL changed the title fix dihedral angle sign fix dihedral angle computation Dec 8, 2023
@lrineau lrineau added the rm only: ready for release branch For the release team only: that indicates that a PR is about to be merged in a release branch label Dec 11, 2023
@lrineau lrineau merged commit b50df6e into CGAL:5.5.x-branch Dec 11, 2023
8 checks passed
lrineau added a commit that referenced this pull request Dec 11, 2023
@lrineau
Merge pull request #7892 from sloriot/CGAL-fix_dh_angle_sign
4be3a1e 
fix dihedral angle computation
lrineau added a commit that referenced this pull request Dec 11, 2023
@lrineau
Merge pull request #7892 from sloriot/CGAL-fix_dh_angle_sign
e2a745f 
fix dihedral angle computation
@lrineau lrineau removed rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' rm only: ready for release branch For the release team only: that indicates that a PR is about to be merged in a release branch labels Dec 11, 2023
@lrineau lrineau deleted the CGAL-fix_dh_angle_sign branch December 11, 2023 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lrineau lrineau lrineau approved these changes

@afabri afabri afabri left review comments

Assignees

@lrineau lrineau

Labels
Bug Merged_in_5.5.4 Merged_in_5.5.5 Merged_in_5.6.1 Merged_in_5.6.2 Merged_in_6.0 Pkg::Kernel_23 Tested
Projects
None yet
Milestone
5.5.4
Development

Successfully merging this pull request may close these issues.
4 participants
@sloriot @afabri @lrineau @MaelRL