-
-
Notifications
You must be signed in to change notification settings - Fork 993
feat: add number/float16/base/from-word
#8733
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
base: develop
Are you sure you want to change the base?
feat: add number/float16/base/from-word
#8733
Conversation
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: passed
- task: lint_package_json
status: passed
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: passed
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: passed
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: passed
- task: lint_license_headers
status: passed
---
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Coverage Report
The above coverage report was generated for the changes in this PR. |
|
/stdlib update-copyright-years |
| var MANTISSA_MASK = 0x03FF; // 0x03FF = 1023 => 0 00000 1111111111 | ||
|
|
||
| var NUM_MANTISSA_BITS = 10; // Number of mantissa bits in float16 | ||
| var MAX_EXPONENT = 31; // Maximum exponent value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of follow-up PRs: similar to constants/float64 and constants/float32, we should move these constants to standalone packages in the constants/float16 namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find any constant similar to MAX_EXPONENT. Do something similar exist in constants/float[32/64] or should I create it with name constants/float16/max-exponent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the max-base(10|2)-exponent packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are present for both float32 and float64 and may be what you are looking for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FLOAT16_MAX_BASE2_EXPONENT would return 15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the base 10 exponent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
base 10 exponent would return 4.
Actually we can take this as
var MAX_EXPONENT = pow( 2, FLOAT16_NUM_EXPONENT_BITS ) - 1; There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me. Although, essentially, 31 = MAX_BASE2_EXPONENT*2 + 1. Not sure if that helps convey the meaning/significance here of 31. I suppose the intent here is what is the value of 11111, which is 31, so your formula works. Although, I would recommend using exp2, rather than pow.
lib/node_modules/@stdlib/number/float16/base/from-word/lib/main.js
Outdated
Show resolved
Hide resolved
|
/stdlib merge |
|
Need #8771 to be merged first |
|
/stdlib merge |
lib/node_modules/@stdlib/number/float16/base/from-word/README.md
Outdated
Show resolved
Hide resolved
| var arr = uniform( 1000, 0.0, MAX_UINT16 ); | ||
|
|
||
| // Round each number: | ||
| var word = map( arr, naryFunction( round, 1 ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make more sense to just use random/array/discrete-uniform instead? Then you don't need two separate steps.
| var y; | ||
| var i; | ||
|
|
||
| x = uniform( 100, 0.0, MAX_UINT16 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing regarding discrete-uniform.
| var fromWord = require( './../lib' ); | ||
|
|
||
| // Generate an array of random numbers: | ||
| var arr = uniform( 1000, 0.0, MAX_UINT16 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as README.
| return 0.0; | ||
| } | ||
| // Subnormal number: (-1)^sign × 2^(-14) × (0.mantissa) | ||
| f16 = pow( 2.0, SUBNORMAL_EXPONENT ) * ( mantissa / 1024.0 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of pow( 2.0, exponent ), I suggest using exp2 which is specifically tailored to 2^exp operations.
Ref: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/math/base/special/exp2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applies here and below.
| // VARIABLES // | ||
|
|
||
| var MAX_EXPONENT = 31; // Maximum exponent value | ||
| var SUBNORMAL_EXPONENT = 1 - FLOAT16_EXPONENT_BIAS; // Minimum exponent for subnormal numbers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we have an equivalent constant in float32/float64 for this, correct? Meaning, is this another opportunity to move into a separate package?
|
|
||
| // VARIABLES // | ||
|
|
||
| var MAX_EXPONENT = 31; // Maximum exponent value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed elsewhere, this can become exp2( num_exponent_bits ) - 1.
lib/node_modules/@stdlib/number/float16/base/from-word/docs/repl.txt
Outdated
Show resolved
Hide resolved
Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Resolves None.
Description
This pull request:
number/float16/base/from-wordRelated Issues
This pull request has the following related issues:
Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
{{TODO: add disclosure if applicable}}
@stdlib-js/reviewers