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

Add a Remainder method #44

Merged
merged 13 commits into from Sep 10, 2019
Merged

Add a Remainder method #44

merged 13 commits into from Sep 10, 2019

Conversation

ghost
Copy link

@ghost ghost commented Sep 8, 2019

Adds a vectorized remainder method that mirrors the C# implementation (not the IEEE one).
Also adds a Truncate method which rounds towards zero unlike the current Round method which rounds towards the nearest integer.
This remainder method is an approximation and does not always equal the C# %, it is however very close.

TODO:

  • Add the method
  • Tests?
  • Benchmarks

Consider:

  • Add an IEEERemainder?

@john-h-k
Copy link
Owner

john-h-k commented Sep 8, 2019

Sounds good to me. Not sure how to implement it at first glance tho. Would recommend looking at DirectXMath source code for inspiration

@ghost ghost changed the title [WIP] Add a Remainder method Add a Remainder method Sep 8, 2019
@ghost
Copy link
Author

ghost commented Sep 8, 2019

Ready for review.

Current benchmark results:
image

@john-h-k
Copy link
Owner

john-h-k commented Sep 8, 2019

Can you modify the test to follow the format of the other ones (no output)? And add a little more test data. Some normal inputs, NaN, ±infinity etc

@ghost
Copy link
Author

ghost commented Sep 8, 2019

Sure, the output was mostly to make sure everything is actually working properly.
I'll also add some more cases.

@ghost
Copy link
Author

ghost commented Sep 8, 2019

Currently there seems to be issues related to precision which result in weird unexpected results for higher values. I'll have to trace down the actual CLR implementation of %.

@ghost
Copy link
Author

ghost commented Sep 8, 2019

Ready for another review.
Current benchmark results:

|    Method |      Mean |     Error |    StdDev |
|---------- |----------:|----------:|----------:|
|    Normal | 67.287 ns | 0.4462 ns | 0.4174 ns |
| MathSharp |  2.729 ns | 0.0411 ns | 0.0384 ns |

Copy link
Owner

@john-h-k john-h-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appears good to me. Reviewing it from my CS class but I think its thorough enough

@john-h-k john-h-k merged commit 75ce8f8 into john-h-k:master Sep 10, 2019
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.

None yet

2 participants