-
-
Notifications
You must be signed in to change notification settings - Fork 68
Implementation of Unum 2.0 #482
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: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughReplaces the parametric unum2 number type with a new lattice-indexed design. The old template parameterized by exponent size, fraction size, and bit type is replaced with a simpler index-based model using a separate lattice template. The lattice manages exact value representations, while unum2 now stores indices into that lattice and delegates arithmetic to lattice-based operations. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant OldUnum2 as unum2 (old)<br/>esizesize, fsizesize, bt
participant NewUnum2 as unum2 (new)<br/>S, T
participant Lattice
rect rgb(230, 245, 255)
Note over OldUnum2: Old: Value-oriented
User->>OldUnum2: new unum2(3.14)
OldUnum2->>OldUnum2: Store raw bits
User->>OldUnum2: operator+=(2.71)
OldUnum2->>OldUnum2: Direct arithmetic
end
rect rgb(240, 255, 240)
Note over NewUnum2: New: Index-based
User->>NewUnum2: from(3.14)
NewUnum2->>Lattice: Lookup exact value
Lattice-->>NewUnum2: Return index
NewUnum2->>NewUnum2: Store index
User->>NewUnum2: operator+(other)
NewUnum2->>Lattice: Get exact(index1)
NewUnum2->>Lattice: Get exact(index2)
Lattice-->>NewUnum2: Exact values
NewUnum2->>Lattice: Find sum result
Lattice-->>NewUnum2: Return result index
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Ravenwater
left a comment
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.
did a quick scan but didn't look at functionality yet. couple of quick notes, and please rebase on branch v3.87 and PR to that branch.
| #pragma once | ||
| // unum2.hpp: definition of the flexible configuration universal number system | ||
| // | ||
| // Copyright (C) 2017-2021 Stillwater Supercomputing, Inc. |
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.
need the more modern copyright header that is being used throughout the rest of the library:
// Copyright (C) 2017 Stillwater Supercomputing, Inc.
// SPDX-License-Identifier: MIT
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.
This is the old unum2 header file. We can ignore this. As for the copyright header, I will be adding it once I finish up, as the code [and the files associated] is now stable.
|
|
||
| // modifiers | ||
|
|
||
| /// <summary> |
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.
remove the commentary for simple modifiers and selectors. These are header files and thus all the characters in the file are read during compilation. No need to document standard APIs that have a well defined meaning.
|
@singul4ri7y The repo has changed significantly (mostly additive) since you first submitted this. We are now on dev branch v3.90. What I suggest we do is go back to PR this to the main branch as you will get the CodeRabbit review feedback. But since the main has moved three dev branches since you started, make certain you rebase your code on the latest main branch. Then issue the PR and we'll work through the review. We need to finish this process in a week or so otherwise your PR is going to diverge again. So make certain you have the time to follow through on the CodeRabbit suggestions. Looking forward to your reply. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
include/sw/universal/number/unum2/_unum2_impl.hpp(1 hunks)include/sw/universal/number/unum2/lattice.hpp(1 hunks)include/sw/universal/number/unum2/unum2_impl.hpp(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-08-25T12:54:12.489Z
Learnt from: Ravenwater
Repo: stillwater-sc/universal PR: 477
File: include/sw/universal/number/td/attributes.hpp:20-31
Timestamp: 2025-08-25T12:54:12.489Z
Learning: The header include/sw/universal/number/td/attributes.hpp is an internal header that is NOT to be used outside of the library. Missing standard library includes like <string>, <sstream>, and <iomanip> are intentional as this header is always included through other headers that provide these dependencies.
Applied to files:
include/sw/universal/number/unum2/lattice.hppinclude/sw/universal/number/unum2/_unum2_impl.hpp
📚 Learning: 2025-08-22T18:06:33.669Z
Learnt from: Ravenwater
Repo: stillwater-sc/universal PR: 475
File: include/sw/universal/verification/test_suite_mathlib.hpp:44-45
Timestamp: 2025-08-22T18:06:33.669Z
Learning: The test suite in include/sw/universal/verification/test_suite_mathlib.hpp is designed for number systems with nbits <= 12, so the expression (1 << nbits) for NR_TEST_CASES will never cause integer overflow in practice.
Applied to files:
include/sw/universal/number/unum2/lattice.hppinclude/sw/universal/number/unum2/unum2_impl.hppinclude/sw/universal/number/unum2/_unum2_impl.hpp
📚 Learning: 2025-09-20T14:28:53.538Z
Learnt from: Ravenwater
Repo: stillwater-sc/universal PR: 481
File: education/internal/blocktriple.cpp:108-129
Timestamp: 2025-09-20T14:28:53.538Z
Learning: In the blocktriple class (include/sw/universal/internal/blocktriple/blocktriple.hpp), the arithmetic methods like add(), mul(), and div() are designed by intent to only handle normal computation paths. Special value handling for NaN, infinity, and zero cases is expected to be dealt with as preconditions before calling these arithmetic operators, not within the operators themselves.
Applied to files:
include/sw/universal/number/unum2/unum2_impl.hppinclude/sw/universal/number/unum2/_unum2_impl.hpp
📚 Learning: 2025-08-22T18:30:36.698Z
Learnt from: Ravenwater
Repo: stillwater-sc/universal PR: 475
File: include/sw/universal/number/bfloat/math/functions/trigonometry.hpp:8-9
Timestamp: 2025-08-22T18:30:36.698Z
Learning: In the Universal Numbers Library, when reviewing individual math function headers under include/sw/universal/number/bfloat/math/functions/, relative include paths like <math/constants/float_constants.hpp> are correct and individual headers should not include <cmath> or the main type header as these dependencies are satisfied at the mathlib aggregator level. This is an intentional architectural design choice.
Applied to files:
include/sw/universal/number/unum2/_unum2_impl.hpp
📚 Learning: 2025-08-24T11:16:57.129Z
Learnt from: Ravenwater
Repo: stillwater-sc/universal PR: 477
File: include/sw/universal/internal/floatcascade/dd_impl.hpp:11-22
Timestamp: 2025-08-24T11:16:57.129Z
Learning: In the universal library, the team is consolidating the old dd (double-double) type with a new floatcascade-based dd implementation in include/sw/universal/internal/floatcascade/dd_impl.hpp. The apparent ODR/name collision is intentional as they are migrating from the old dd to the new floatcascade-based dd type.
Applied to files:
include/sw/universal/number/unum2/_unum2_impl.hpp
📚 Learning: 2025-08-22T18:07:22.725Z
Learnt from: Ravenwater
Repo: stillwater-sc/universal PR: 475
File: include/sw/universal/number/bfloat/math/functions/truncate.hpp:1-9
Timestamp: 2025-08-22T18:07:22.725Z
Learning: In the Universal Numbers Library bfloat16 mathlib, individual function headers under include/sw/universal/number/bfloat/math/functions/ (like truncate.hpp) are designed to be included only through the main mathlib aggregator header, not standalone. Dependencies like <cmath> and bfloat16 headers are intentionally satisfied at the mathlib level rather than in individual function headers. This is an architectural design choice to maintain a clean modular structure.
Applied to files:
include/sw/universal/number/unum2/_unum2_impl.hpp
🪛 Clang (14.0.6)
include/sw/universal/number/unum2/lattice.hpp
[error] 3-3: 'universal/number/unum2/unum2_impl.hpp' file not found
(clang-diagnostic-error)
include/sw/universal/number/unum2/unum2_impl.hpp
[error] 3-3: 'iostream' file not found
(clang-diagnostic-error)
include/sw/universal/number/unum2/_unum2_impl.hpp
[error] 7-7: 'cassert' file not found
(clang-diagnostic-error)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run for windows-latest
- GitHub Check: Run for macos-latest
- GitHub Check: Run for ubuntu-latest
- GitHub Check: Codacy Static Code Analysis
|
We do need the library to support plug-in replacements, so you will need to support _fwd.hpp, all interactions with literal int and float arguments and rhs, and make certain that you don't trigger implicit conversions. Take a look at the other number systems root include file: please follow that structure and provide the implementations of the support structures. Here is an example of the double-doube number system. First pass you can stub out the mathlib. |
|
@Ravenwater Hopefully the newly added code are not associated with the section I am working on. If that is the case then rebase should be pretty straight forward, I will do the rebase as soon as I remove the WIP status (it seems like you can't rebase unless you do so). I am a little bit reluctant to acknowledge CodeRabbit AI's suggestions, as my last PR broke by doing so 😅. But it seems like CR AI got some good suggestions this time. I will fix the the issues (if they are legit issues) ASAP. |
|
Today, did another big PR. Should all be parallel to you in a number system called |
|
@Ravenwater cool! I will check it out. Turns out we got a bug in the interval mechanism, which is causing the multiplication operation to run wild. I will also implement reverse continuous interval while I am at it (this is mostly causing the issue). Also, should I add the examples in the |
|
Perfect.
I have been following the pattern that we have a set of directories in the
./static/<numbersystem>
api/
arithmetic/
logic/
conversion/
To receive the regression tests for that function. In api/ I typically put
an api.cpp that does all the basic operations of the api, such as,
construction, trivial copy construction tests, arithmetic and logic
examples , and comparisons.
More complicated examples that implement and algorithm specific to the
features of the number system could go to either api/ or applications/
depending on the complexity.
./education should have pedagogical examples that showcase what you can do
with the number system.
|
|
@singul4ri7y how are you doing with this PR? |
|
@Ravenwater Turns out the point-to-point multiplication code for non-exacts are completely bugged. We need a paradigm shift here. Currently I am working on adapting some changes to the code. Literature definitions/recap: Point-to-point operation is when we operate between two single unum2 index (exact or non-exact). In SORN operation, according to the original slides from Dr. Gustafson, we take all individual set points in SORNs, do point-to-point operation and bitwise OR them to get resulting SORN. Now, the way we handle point-to-point non-exact operation for addintion and multiplication is:
This works quite well for point-to-point addition, but not so much for multiplication. Multiplication is kinda unique in some way, such as 1 + 1 == 2 but 1 * 1 == 1, for addition, the result point is moving forward but for multiplication it's not. So, in some cases, we are getting same lower and upper bounds, which in turn results an empty SORN. SORN multiplications at this point might seem accurate but at it's core, it's not. I am planning to remove the logic for taking average and calling Also, the unum2 implementation in this paper seems promising. I will ping you when I have some insights. |
|
@singul4ri7y CI is failing due to a missing header file: fatal error: sw/universal/number/unum2/common.hpp: No such file or directory This error occurs during the build of static/unum2/api.cpp, from this line: #include <sw/universal/number/unum2/common.hpp> |
|
@Ravenwater I have double checked and |
This PR contains implementation of Unum 2.0:
Defining union interval (non-continuous SORN)Summary by CodeRabbit
New Features
Refactor