-
Notifications
You must be signed in to change notification settings - Fork 232
Bug fix of get angle #6820
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
base: main
Are you sure you want to change the base?
Bug fix of get angle #6820
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6820 +/- ##
==========================================
- Coverage 79.05% 79.02% -0.02%
==========================================
Files 566 566
Lines 43649 43660 +11
==========================================
- Hits 34501 34499 -2
- Misses 9148 9161 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Muhammad-Rebaal Thanks! I am currently on holiday so will do a more detailed review later, but this will need tests, both for the happy path and for the error conditions.
| else: | ||
| dot_product = numpy.vdot(cell[i], cell[j]) | ||
| cos_angle = dot_product / (lengths[i] * lengths[j]) | ||
| # Handle numerical issues where |cos_angle| might slightly exceed 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to have a test case for this. Have you encountered this yourself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No didn't encountered that , but no worries. I'll write one .
54cce4f to
37670df
Compare
|
If I have time during the coding days, my plan here is to revert unnecessary changes in this PR (e.g. focus only on the new error handling) and write tests. |
Hi @danielhollas !
I've implemented the fix for the issue #6375 with
StructureData.cell_anglesas you requested. The solution now:The implementation does exactly what we discussed in the issue conversation. If there's anything else that should be changed or improved, please let me know.
Thank You!