float== #61
float== #61
Conversation
|
""" WalkthroughThis update modifies function parameter passing in Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant FloatEquals
Note over Caller, FloatEquals: Floating-point equality check
Caller->>FloatEquals: Call float_equals(a, b, tolerance)
FloatEquals->>FloatEquals: Compute |a - b| using std::abs
FloatEquals-->>Caller: Return boolean result
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
inc/mkn/kul/float.hpp (2)
38-40: Add documentation to explain the tolerance value.Consider adding documentation to explain why 1e-6 was chosen as the default tolerance for single-precision floats. This helps users understand if this tolerance is appropriate for their use case.
Add a doc comment like this:
+/** + * Compares two float values for equality within a specified tolerance. + * @param a First float value + * @param b Second float value + * @param diff Tolerance value (default: 1e-6 suitable for most single-precision calculations) + * @return true if the absolute difference between a and b is less than diff + */ auto inline float_equals(float const& a, float const& b, float diff = 1e-6) {
42-44: Add documentation and handle special cases.
- Add documentation to explain the tolerance value.
- Consider handling special cases like NaN and infinity.
Apply these improvements:
+/** + * Compares two double values for equality within a specified tolerance. + * @param a First double value + * @param b Second double value + * @param diff Tolerance value (default: 1e-12 suitable for most double-precision calculations) + * @return true if the absolute difference between a and b is less than diff + */ auto inline float_equals(double const& a, double const& b, double diff = 1e-12) { + // Handle NaN + if (std::isnan(a) || std::isnan(b)) return false; + // Handle infinity + if (std::isinf(a) || std::isinf(b)) return a == b; return std::abs(a - b) < diff; }Don't forget to add the necessary header:
#include <cstdlib> +#include <cmath>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
inc/mkn/kul/assert.hpp(1 hunks)inc/mkn/kul/float.hpp(1 hunks)
🔇 Additional comments (2)
inc/mkn/kul/assert.hpp (2)
55-57: LGTM! Good improvement in parameter type.Changing from
bool const&tobool constis a good improvement since passing a bool by reference has no performance benefit and can make the code less readable.
59-61: LGTM! Good improvement in parameter type.Same improvement as
abort_if- changing frombool const&tobool constis better since bool is already a small type.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
inc/mkn/kul/float.hpp (2)
34-34: Use<cmath>instead of<cstdlib>for mathematical functions.
<cmath>is more appropriate for mathematical functions likestd::abswhen working with floating-point types.-#include <cstdlib> +#include <cmath>
38-40: Consider improvements to the floating-point comparison functions.
- The return type could be more explicit:
-auto inline float_equals(float const a, float const b, float const diff = 1e-6) { +inline bool float_equals(float const a, float const b, float const diff = 1e-6) {
- The functions should handle special cases like NaN and infinity:
inline bool float_equals(float const a, float const b, float const diff = 1e-6) { + if (std::isnan(a) || std::isnan(b)) return false; + if (std::isinf(a) || std::isinf(b)) return a == b; return std::abs(a - b) < diff; }
- Consider adding documentation to explain:
- The purpose of the default tolerance values
- How special cases are handled
- That the comparison is absolute (not relative)
Also applies to: 42-44
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
inc/mkn/kul/assert.hpp(1 hunks)inc/mkn/kul/float.hpp(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- inc/mkn/kul/assert.hpp
🔇 Additional comments (1)
inc/mkn/kul/float.hpp (1)
36-36: LGTM!The namespace declaration uses modern C++ nested namespace syntax.
| #ifndef _MKN_KUL_FLOAT_HPP_ | ||
| #define _MKN_KUL_FLOAT_HPP_ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rename header guard to avoid reserved identifiers.
The current header guard starts with an underscore followed by a capital letter (_MKN_KUL_FLOAT_HPP_), which is reserved for the implementation in C++. Consider renaming it to MKN_KUL_FLOAT_HPP.
-#ifndef _MKN_KUL_FLOAT_HPP_
-#define _MKN_KUL_FLOAT_HPP_
+#ifndef MKN_KUL_FLOAT_HPP
+#define MKN_KUL_FLOAT_HPP📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #ifndef _MKN_KUL_FLOAT_HPP_ | |
| #define _MKN_KUL_FLOAT_HPP_ | |
| #ifndef MKN_KUL_FLOAT_HPP | |
| #define MKN_KUL_FLOAT_HPP |
|
|
||
| } // namespace mkn::kul | ||
|
|
||
| #endif /* _MKN_KUL_FLOAT_HPP_ */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update the end header guard comment to match the new header guard name.
For consistency with the earlier header guard change:
-#endif /* _MKN_KUL_FLOAT_HPP_ */
+#endif /* MKN_KUL_FLOAT_HPP */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #endif /* _MKN_KUL_FLOAT_HPP_ */ | |
| #endif /* MKN_KUL_FLOAT_HPP */ |
Summary by CodeRabbit
New Features
Refactor