[update] Use `ceil` instead of `%` operator to normalize angle

[AlexanderFabisch]

@luckykk273 thanks again for your suggestion. At the moment I'm not completely sure which version would be the best, see my inline comment.

[AlexanderFabisch]

Two more remarks:

• Please rebase on the current develop branch.
• Consider adding more unit tests to check the behavior that you explained for very small numbers, large numbers, etc.

[luckykk273]

Hi @AlexanderFabisch, I just wanted to kindly remind you that all things mentioned earlier have already been changed. And I have already synchronized with the latest progress of the develop branch. Thank you again for your time and all the suggestions! Feel free to ask any questions if there is any problem!

[AlexanderFabisch]

@luckykk273 I think then we are ready to merge. However, the rebase did not work. Maybe you rebased on an old version of the develop branch? Could you try to update the develop branch first and then rebase on it?

[luckykk273]

@AlexanderFabisch Can you please explain in more detail what "the rebase did not work" means? I also met the same problem you mentioned because in the begining this branch had conflicts with the develop branch but I have already merged the latest version of the develop branch manually yesterday at commit f3d75fc and commit a58e7ec. Currently, on my page, there are only 3 file changes (all related to norm_angle), and it shows that there are no conflicts with the develop branch.

[AlexanderFabisch]

but I have already merged the latest version of the develop branch manually yesterday at commit f3d75fc and commit a58e7ec.

OK, this is the problem then. I was assuming you did a rebase:

git fetch origin
git rebase origin/develop

This will append your commits to the develop branch. So that it appears as if your commits were done later.

The history looks a bit confusing when you merge from develop to a feature branch and it seems like that happened twice here.

However, I see that GitHub now displays the diff between develop and this PR correctly. I'll just have a look if I can merge it with a cleaner history.

[AlexanderFabisch]

However, I see that GitHub now displays the diff between develop and this PR correctly. I'll just have a look if I can merge it with a cleaner history.

FYI, I did an interactive rebase with

git rebase -i develop

and deleted all commits that came from the merge.

Unfortunately, I also had to decrease the number of tested angles to 10^6, which speeds up the tests by 3 seconds on my computer (11 -> 8 seconds). It is important that unit tests are fast because otherwise nobody really executes them anymore.

Thanks for your contribution! It will be part of the next release.

[luckykk273]

The history looks a bit confusing when you merge from develop to a feature branch and it seems like that happened twice here.

FYI, I did an interactive rebase with git rebase -i develop deleted all commits that came from the merge.

Thank you so much for helping clarify and correct my mistake!

Unfortunately, I also had to decrease the number of tested angles to 10^6, which speeds up the tests by 3 seconds on my computer (11 -> 8 seconds).

FYR, it is okay to decrease the number of tested angles and if ones really wants to test the precision of float64, I think we can test it manually as follow:

# test precision of float64 manually
a_norm = np.array([
    (2.0 * np.pi) / 1e16,
    -(2.0 * np.pi) / 1e16,
    (2.0 * np.pi) / 1e16 + np.pi * 0.5,
    -(2.0 * np.pi) / 1e16 + np.pi * 0.5,
    (2.0 * np.pi) / 1e16 - np.pi * 0.5,
    -(2.0 * np.pi) / 1e16 - np.pi * 0.5,
], dtype=np.float64)
for b in np.linspace(-10.0 * np.pi, 10.0 * np.pi, 11):
    a = a_norm + b
    assert_array_almost_equal(pr.norm_angle(a), a_norm)

Thank you again for your time!

[AlexanderFabisch]

Thanks for the clarification!