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

Test the trigonometry routine on remarkable angles #164

Merged
merged 4 commits into from
May 31, 2019

Conversation

nbraud
Copy link
Collaborator

@nbraud nbraud commented May 31, 2019

  • Compare the remarkable angles to Vector._trig, and not cos, sin, in test_remarkable_angles.
  • Tighten-up the tolerances on that test.
  • Modify utils.isclose to be usable outside Hypothesis.
  • Modify rotate/remarkable_angles to use the same convention as Vector._trig.

nbraud added 4 commits May 30, 2019 20:28
The trigonometric values for those angles are known exactly
(expressed in terms of basic arithmetic and square roots).
This precludes using it outside an Hypothesis test,
like in rotate/test_remarkable_angles.
This is the same convention used by Vector2._trig,
which makes things a bit clearer and more convenient.
@nbraud
Copy link
Collaborator Author

nbraud commented May 31, 2019

I mentioned on Discord that Vector._trig was producing values that were off (when compared to remarkable_angles). The initial test was actually buggy: remarkable_angles contained (sin, cos) pairs, whereas Vector._trig returns (cos, sin) pairs.

@nbraud nbraud mentioned this pull request May 31, 2019
@nbraud nbraud changed the title Test the trigonometry routing on remarkable angles Test the trigonometry routine on remarkable angles May 31, 2019
@AstraLuma
Copy link
Member

I should note that if we have imprecision/inconsistency in these angles, I think we should take the answer that gives us smooth curves than what's 100% correct.

bors r+

bors bot added a commit that referenced this pull request May 31, 2019
164: Test the trigonometry routine on remarkable angles r=astronouth7303 a=nbraud

- [x] Compare the remarkable angles to `Vector._trig`, and not `cos, sin`, in `test_remarkable_angles`.
- [x] Tighten-up the tolerances on that test.
- [x] Modify `utils.isclose` to be usable outside Hypothesis.
- [x] Modify `rotate/remarkable_angles` to use the same convention as `Vector._trig`.

Co-authored-by: Nicolas Braud-Santoni <nicolas@braud-santoni.eu>
@nbraud
Copy link
Collaborator Author

nbraud commented May 31, 2019

@astronouth7303 What do you mean by “smooth curves“ here ?

In any case, rotations should be “smooth” for whatever reasonable meaning of it; I can probably write a test to check they are 1-Lipschitz.

@AstraLuma
Copy link
Member

I just mean, I don't want any perturbances or "bumps" or jitter caused by certain angles being hardcoded or whatever.

@bors
Copy link
Contributor

bors bot commented May 31, 2019

Build succeeded

  • docs
  • FreeBSD PYTHON:3.6
  • FreeBSD PYTHON:3.7
  • lint
  • Linux python:3.6-slim
  • Linux python:3.7-slim
  • macOS PYTHON:3.6.8
  • macOS PYTHON:3.7.2
  • Windows python:3.6-windowsservercore-1809
  • Windows python:3.7-windowsservercore-1809

@bors bors bot merged commit 923493d into ppb:master May 31, 2019
@nbraud
Copy link
Collaborator Author

nbraud commented Jun 1, 2019

@astronouth7303 Oh, yes; only the test uses known values, not the implementation.

@nbraud nbraud deleted the tests/trig branch June 1, 2019 08:17
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.

2 participants