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

feat: add C implementation for math/base/special/round #745

Closed
wants to merge 13 commits into from

Conversation

Pranavchiku
Copy link
Member

@Pranavchiku Pranavchiku commented Dec 20, 2022

Resolves #735.

Checklist

  • update readme.md
  • include.gypi
  • binding.gyp
  • include/stdlib/math/base/special/
  • src
  • manifest.json
  • lib
  • examples
  • benchmark
  • test

@kgryte

@Pranavchiku Pranavchiku marked this pull request as ready for review December 20, 2022 07:06
@Pranavchiku
Copy link
Member Author

In test.native.js I removed one test t.strictEqual( round( -4.5 ), -4.0, 'equals -4' ); as for

Javascript

Math.round( -4.5 ) = -4

C

round( -4.5 ) = -5

Rest all tests passed and benchmarks cleared, this PR can have a review.

@kgryte
Copy link
Member

kgryte commented Dec 20, 2022

@Pranavchiku This is an instance where we want to emulate JS behavior in C, I believe. Or maybe instead, we should do whatever IEEE 754-2008 specifies. Would be worth reading the spec and seeing what the default round behavior is specified to be. IIRC correctly, JavaScript is not IEEE 754 compliant, but this should be confirmed.

@Pranavchiku
Copy link
Member Author

Okay, I will have a look at it.

@Planeshifter Planeshifter changed the title Add C implementation for @stdlib/math/base/special/round feat: add C implementation for math/base/special/round Mar 3, 2024
@Shashankss1205
Copy link
Contributor

Accordin to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/round:
"If the fractional portion is exactly 0.5, the argument is rounded to the next integer in the direction of +∞."

So I am confused if the rounding is to be done, then what rule will it follow?

@kgryte
Copy link
Member

kgryte commented Mar 3, 2024

@Shashankss1205 What is the confusion? In short, JS behavior is to round ties in a manner similar to ceil. In all other cases, round to the nearest integer. E.g.,

var v = round( 1.2 );
// returns 1.0

v = round( 1.8 );
// returns 2.0

v = round( 1.5 );
// returns 2.0

v = round( -1.2 );
// returns -1.0

v = round( -1.8 );
// returns -2.0

v = round( -1.5 );
// returns 1.0

Notice the behavior of 1.5 and -1.5 above.

@Shashankss1205
Copy link
Contributor

@Shashankss1205 What is the confusion? In short, JS behave is to round ties in a manner similar to ceil. In all other cases, round to the nearest integer. E.g.,

var v = round( 1.2 );
// returns 1.0

v = round( 1.8 );
// returns 2.0

v = round( 1.5 );
// returns 2.0

v = round( -1.2 );
// returns -1.0

v = round( -1.8 );
// returns -2.0

v = round( -1.5 );
// returns -1.0

Notice the behavior of 1.5 and -1.5 above.

Yeah understood, Thanks for confirming!

@Pranavchiku
Copy link
Member Author

IEEE 754-2008

The roundTiesToEven rounding-direction attribute shall be the default rounding-direction attribute for results in binary formats. The default rounding-direction attribute for results in decimal formats is language-defined, but should be roundTiesToEven.

rountTiesToEven

roundTiesToEven, the floating-point number nearest to the infinitely precise result shall be delivered; if the two nearest floating-point numbers bracketing an unrepresentable infinitely precise result are equally near, the one with an even least significant digit shall be delivered

Source: https://iremi.univ-reunion.fr/IMG/pdf/ieee-754-2008.pdf

@Pranavchiku
Copy link
Member Author

We can continue discussion at #1450, pushing implementation from https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-math.round.

@kgryte kgryte added Duplicate This issue or pull request already exists. Feature Issue or pull request for adding a new feature. Math Issue or pull request specific to math functionality. C Issue involves or relates to C. labels Mar 15, 2024
@Planeshifter
Copy link
Member

Subsequently merged in #1450.

@Pranavchiku Pranavchiku mentioned this pull request Aug 15, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Issue involves or relates to C. Duplicate This issue or pull request already exists. Feature Issue or pull request for adding a new feature. Math Issue or pull request specific to math functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC]: Add C implementation for @stdlib/math/base/special/round
4 participants