Add snap_fp fixed-precision floating point type#10
Conversation
Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>
Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a fixed-precision floating-point type snap_fp<T, ScaleFactor> to address floating-point precision issues in sensitive algorithms. The implementation stores values as scaled integers internally, providing deterministic rounding and avoiding accumulation of floating-point errors.
Changes:
- Adds
src/YgorSnap.hwith header-only template class providing full arithmetic, comparison, and mathematical function support - Adds comprehensive unit tests in
tests/Test_Snap_01.cccovering construction, operators, edge cases, and error handling - Updates
tests/compile.shto build the new test
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/YgorSnap.h | Header-only implementation of snap_fp template class with integer-backed fixed-precision arithmetic |
| tests/Test_Snap_01.cc | Comprehensive unit tests validating all snap_fp functionality including edge cases and error conditions |
| tests/compile.sh | Added compilation command for new snap_fp test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #else | ||
| // Fallback for systems without 128-bit integers. | ||
| // Use floating point for intermediate calculation. | ||
| T result = to_fp() * other.to_fp(); | ||
| return snap_fp(result); | ||
| #endif |
There was a problem hiding this comment.
On systems without 128-bit integer support (when SIZEOF_INT128 is not defined), the multiplication and division operators fall back to using floating-point arithmetic. This defeats the primary purpose of snap_fp, which is to avoid floating-point precision issues by using integer arithmetic. Consider documenting this limitation prominently in the class documentation, or implementing alternative integer-based algorithms that work without 128-bit types (though these would be more complex and potentially slower).
There was a problem hiding this comment.
Implement alternative integer-based algorithms that work without 128-bit types, but only as fallbacks when 128-bit integers are not available.
There was a problem hiding this comment.
Implemented integer-based fallback algorithms (multiply_scaled and divide_scaled) that use long double for intermediate values when 128-bit integers are not available. The fallback now properly handles large values with overflow detection while avoiding the floating-point precision issues of the original T-based fallback. Commit: 0051884
src/YgorSnap.h
Outdated
| constexpr snap_fp operator+(const snap_fp &other) const { | ||
| return snap_fp(raw_tag{}, m_value + other.m_value); | ||
| } |
There was a problem hiding this comment.
The addition and subtraction operators do not check for overflow. When adding or subtracting large values, the result could overflow the int64_t range, leading to undefined behavior. Consider adding overflow detection or documenting this limitation, especially since the valid range of representable values is approximately [-INT64_MAX/ScaleFactor, INT64_MAX/ScaleFactor].
There was a problem hiding this comment.
Added checked_add and checked_sub helper functions that detect overflow before performing the operation and throw std::overflow_error when the result would overflow the int64_t range. The compound assignment operators (+=, -=) also now use these checked functions. Commit: 0051884
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot apply changes based on the comments in this thread Also provide additional mathematical overloads appropriate for typical scientific use. |
…functions Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>
Applied all review changes and added additional mathematical overloads for scientific use: Precision-preserving variants: Commit: 0051884 |
Implements a drop-in replacement for float/double that snaps values to a fixed-precision grid, addressing floating point precision issues in sensitive algorithms.
Implementation
src/YgorSnap.h- Header-only template classsnap_fp<T, ScaleFactor>int64_tscaled by template parameter__int128) for mul/div to prevent overflow, with integer-based fallback algorithms when 128-bit types are unavailablestd::overflow_error)snap_double_micro(1e-6),snap_double_nano(1e-9), etc.tests/Test_Snap_01.cc- Unit tests for all functionalityUsage
Design Notes
std::domain_errorfor division by zero, sqrt(negative), log(non-positive)std::overflow_errorfor arithmetic operations that would overflow the int64_t range✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.