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 math/base/special/powf #5739

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

Neerajpathak07
Copy link
Contributor

@Neerajpathak07 Neerajpathak07 commented Mar 2, 2025

Progresses #649.

Description

What is the purpose of this pull request?

This pull request:

  • adds math/base/special/powf which is the single-precision version of pow

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.


@stdlib-js/reviewers

@stdlib-bot stdlib-bot added the Math Issue or pull request specific to math functionality. label Mar 2, 2025
@stdlib-bot
Copy link
Contributor

Hello! 👋

I've noticed that your commit doesn't contain the expected YAML metadata blocks. This typically happens when your development environment isn't properly set up with the stdlib git hooks.

Here's how to fix this:

  1. Install project dependencies (run this command in the top-level directory of the project):

    make install
  2. Initialize the development environment (this sets up the Git hooks among other things):

    make init

If you're still having issues, please check our development guide for more information.

Thank you for your contribution!

var polyvalL = require( './polyval_l.js' );


// VARIABLES //
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these values have been based on upstream implementation of powf in msun.

@Neerajpathak07
Copy link
Contributor Author

@kgryte @gunjjoshi what do you guys think of the JS implementation so far!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have kept this empty as of now thinking of generating fixtures first and get back to this.

@Neerajpathak07
Copy link
Contributor Author

Neerajpathak07 commented Mar 5, 2025

Updates so far cc: @kgryte @gunjjoshi

  • Went through the binary format for float32 as discussed in the meet and referred documentation for to-uint32, from-binary-string & to-binary-string also studied the documentation for constants/float32 packages.
  • Changed var names across logx logax and pow2 with accurate hexadecimal values based on upstream implementation with corresponding logical var names.
  • Based all the JS implementation wrt to powf
  • used JS var like towordf and forWordf to resonate with GET_FLOAT_WORD & SET_FLOAT_WORD.
  • reduced the LOG_WORKSPACE from an array of high and low bits to a singular var to compensate 32-bit int!!
  • wrapped the implementation in float64ToFloat32
  • reduced CI errors now most of them require C implementation of powf
  • updated reference link in JS licenses to the msun file for powf
  • removed the logic for high and low words space and added logic for single variable workspace in main.js

Upcoming updates

  • adding test fixtures.
  • Working on test cases.
  • adding C implementations once the JS one is approved.
  • Fixing bugs and typo errors.

Comment on lines +294 to +295
if ( sn === 1 ) {
// Signal overflow...
return sn * HUGE * HUGE;
}
// Signal underflow...
return sn * TINY * TINY;
Copy link
Contributor Author

@Neerajpathak07 Neerajpathak07 Mar 5, 2025

Choose a reason for hiding this comment

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

@gunjjoshi was a little confused about handling Over and underflow when x is not close to unity

@Neerajpathak07
Copy link
Contributor Author

Neerajpathak07 commented Mar 6, 2025

Had to rebase a branch as an unwanted change was added Ik it's unethical to do this but it was messing the purpose of this big-PR everything is running clean now working on more features.

*
* ## Notice
*
* The following copyright and license were part of the original implementation available as part of [FreeBSD]{@link https://svnweb.freebsd.org/base/release/9.3.0/lib/msun/src/e_powf.c}. The implementation follows the original, but has been modified for JavaScript.
Copy link
Member

Choose a reason for hiding this comment

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

We can update this 9.3.0 version to use the latest implementation.

Copy link
Member

Choose a reason for hiding this comment

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

/**
* Evaluates the exponential function for single-precision values.
*
* ## Method
Copy link
Member

Choose a reason for hiding this comment

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

Method is not required here. We can remove this, as it is not present in the reference implementation.

* x^y = 2^n e^{y^\prime \cdot \mathrm{log2}}
* ```
*
* ## Special Cases
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, this and the Notes section below can be removed too.

Comment on lines 207 to 208
hx = fromWordf( x );
hy = fromWordf( y );
Copy link
Member

Choose a reason for hiding this comment

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

@Neerajpathak07 hx and hy are words, which we are getting from the numbers x and y. So, we should be using toWordf here, instead of fromWordf.

@gunjjoshi
Copy link
Member

@Neerajpathak07 Let's follow https://svnweb.freebsd.org/base/release/12.2.0/lib/msun/src/e_powf.c?revision=367086&view=markup here. The current implementation in main.js looks a bit off from this reference. You might want to restructure it, so that it matches with the reference.

}

// Special cases `x`...
if ( lx === 0 ) {
Copy link
Member

Choose a reason for hiding this comment

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

Where is this lx coming from?

Copy link
Member

Choose a reason for hiding this comment

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

Seems like you've done a port of the double-precision implementation. Ideally, we must be independently following the single-precision reference, as the implementations may vary significantly.

Copy link
Contributor Author

@Neerajpathak07 Neerajpathak07 Mar 7, 2025

Choose a reason for hiding this comment

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

@gunjjoshi I haven't started refactoring the main.js yet was working on log2ax, logx and pow2 to get them in line with single-precision implementation and wanted to get approvals on the var names and hexadecimal values first.
What I have changed in main.js is:-

  • Removed the logic for high and low words.
  • Align the var with correct package convention.

Copy link
Member

Choose a reason for hiding this comment

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

@Neerajpathak07 I would have started the implementation from main.js, referring to the freeBSD implementation, rather than first working on log2ax, logx and pow2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted I'll keep that in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Math Issue or pull request specific to math functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants