more edge values #14
more edge values #14
Conversation
📝 WalkthroughWalkthroughExpanded test edge-case datasets (integers and floats), adjusted complex-math magnitude and logarithm internals for numeric stability, tightened Python-exception mapping in a test helper, and added Changes
Sequence Diagram(s)(omitted — changes are internal numeric/stability improvements and test-data expansions without a multi-component sequential flow requiring visualization) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/math/integer.rs (1)
718-773: Fix array size mismatch causing compilation failure.The pipeline failure indicates the array declaration specifies 44 elements but 45 are provided. Update the type to
[i64; 45]to match the actual element count.🐛 Proposed fix
- const EDGE_I64: [i64; 44] = [ + const EDGE_I64: [i64; 45] = [
🧹 Nitpick comments (1)
src/test.rs (1)
61-65: Duplicate value in array.
-0.9999999999999999appears twice: once at line 62 (atanh boundary) and again at line 64 (log1p boundary). Consider replacing one with a distinct value or removing the duplicate to maximize test coverage with unique values.♻️ Suggested fix - replace duplicate with a distinct value
// log1p domain boundary (> -1) - -0.9999999999999999, // just above -1 + -0.99999999999999, // slightly further from -1 -1.0 + 1e-15, // very close to -1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignoresrc/math/integer.rssrc/test.rs
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Files:
src/test.rssrc/math/integer.rs
src/**/*test*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*test*.rs: All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
Tests must verify both correct values for Ok results and correct error types (EDOM vs ERANGE) for Err results. Python ValueError maps to Error::EDOM and OverflowError maps to Error::ERANGE.
Files:
src/test.rs
src/math/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/math/**/*.rs: Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Files:
src/math/integer.rs
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
Applied to files:
src/test.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : Tests must verify both correct values for Ok results and correct error types (EDOM vs ERANGE) for Err results. Python ValueError maps to Error::EDOM and OverflowError maps to Error::ERANGE.
Applied to files:
src/test.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*.rs : Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Applied to files:
src/test.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/exponential.rs : Copy CPython's explicit special case handling for IEEE specials (NaN, Inf, etc.) exactly, including logic for cases like NaN**0 = 1.
Applied to files:
src/test.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math.rs : Create missing helpers if CPython has a helper we don't have yet, such as math_1_fn for Rust function pointers or special case handlers for specific functions.
Applied to files:
src/test.rssrc/math/integer.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Applied to files:
src/math/integer.rs
🪛 GitHub Actions: Rust
src/math/integer.rs
[error] 718-718: mismatched types: expected an array with a size of 44, found one with a size of 45 for EDGE_I64. Please adjust the array length to 44 to match the actual number of elements.
🔇 Additional comments (3)
.gitignore (1)
3-3: LGTM!Adding
cpythonto.gitignoreis appropriate for ignoring a local CPython checkout used for testing purposes.src/math/integer.rs (1)
719-773: Good expansion of edge case coverage.The expanded
EDGE_I64values provide comprehensive coverage including:
- Table boundaries (127/128 for comb/perm lookup tables)
- Algorithm switching points (34/35 for
FAST_COMB_LIMITS1)- Square root boundaries for different code paths
- Factorial overflow thresholds
The test implementations properly verify both Ok values and error types (EDOM for negative inputs). Based on learnings, this aligns with the testing guidelines.
src/test.rs (1)
6-91: Comprehensive edge value coverage.The expanded
EDGE_VALUESarray now provides excellent coverage as per coding guidelines:
- ✓ Zeros (positive and negative)
- ✓ Infinities
- ✓ NaNs with different payloads (including signaling NaN)
- ✓ Subnormals
- ✓ Boundary values (MIN_POSITIVE, MAX, MIN)
- ✓ Large values near infinity
- ✓ Trigonometric special values (PI, PI/2, PI/4, TAU)
- ✓ Domain boundaries for asin/acos, atanh, log1p, gamma
The signaling NaN inclusion with the platform-specific exception warning is appropriately documented.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/cmath.rs (1)
181-195: Assert error-type mapping when CPython raises.When Python raises ValueError/OverflowError and Rust returns Err, the test currently doesn’t verify EDOM vs ERANGE. Add explicit checks so error-type mismatches are caught.
🔧 Suggested fix
Err(e) => { // CPython raised an exception - check we got an error too - if let Ok(rs) = rs_result { - // Some special cases may return values for domain errors in Python - // Check if it's a domain error - if e.is_instance_of::<pyo3::exceptions::PyValueError>(py) { - panic!( - "{func_name}({re}, {im}): py raised ValueError but rs=({}, {})", - rs.re, rs.im - ); - } else if e.is_instance_of::<pyo3::exceptions::PyOverflowError>(py) { - panic!( - "{func_name}({re}, {im}): py raised OverflowError but rs=({}, {})", - rs.re, rs.im - ); - } - } + match rs_result { + Ok(rs) => { + // Some special cases may return values for domain errors in Python + if e.is_instance_of::<pyo3::exceptions::PyValueError>(py) { + panic!( + "{func_name}({re}, {im}): py raised ValueError but rs=({}, {})", + rs.re, rs.im + ); + } else if e.is_instance_of::<pyo3::exceptions::PyOverflowError>(py) { + panic!( + "{func_name}({re}, {im}): py raised OverflowError but rs=({}, {})", + rs.re, rs.im + ); + } + } + Err(err) => { + if e.is_instance_of::<pyo3::exceptions::PyValueError>(py) { + assert!( + matches!(err, crate::Error::EDOM), + "{func_name}({re}, {im}): py ValueError but rs error {err:?}" + ); + } else if e.is_instance_of::<pyo3::exceptions::PyOverflowError>(py) { + assert!( + matches!(err, crate::Error::ERANGE), + "{func_name}({re}, {im}): py OverflowError but rs error {err:?}" + ); + } + } + } // Both raised errors - OK }Based on learnings, tests must validate error-type mapping.
src/cmath/misc.rs (1)
197-210: Test doesn't verify error type mapping when both Python and Rust fail.Per coding guidelines, tests must verify correct error types:
ValueError→Error::EDOM,OverflowError→Error::ERANGE. The current code only panics when Python raises an error but Rust succeeds. When both fail, it should verify the error types match.🔧 Suggested fix
Err(e) => { // Python raised an exception - check we got an error too - if let Ok(rs_val) = rs_result { - if e.is_instance_of::<pyo3::exceptions::PyValueError>(py) { - panic!("phase({re}, {im}): py raised ValueError but rs={rs_val}"); - } else if e.is_instance_of::<pyo3::exceptions::PyOverflowError>(py) { - panic!("phase({re}, {im}): py raised OverflowError but rs={rs_val}"); - } + if e.is_instance_of::<pyo3::exceptions::PyValueError>(py) { + assert_eq!( + rs_result.err(), + Some(Error::EDOM), + "phase({re}, {im}): py raised ValueError but rs={rs_result:?}" + ); + } else if e.is_instance_of::<pyo3::exceptions::PyOverflowError>(py) { + assert_eq!( + rs_result.err(), + Some(Error::ERANGE), + "phase({re}, {im}): py raised OverflowError but rs={rs_result:?}" + ); + } else { + panic!("phase({re}, {im}): py raised unexpected error {e}"); } - // Both raised errors - OK }src/test.rs (1)
9-103: Duplicate value in EDGE_VALUES.The value
-0.9999999999999999appears twice: once at line 72 (atanh domain boundary) and again at line 74 (log1p domain boundary). This creates redundant test iterations.Based on learnings, the expanded coverage is comprehensive and correctly includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values.
🔧 Suggested fix - remove duplicate
// atanh domain boundaries (-1, 1) 0.9999999999999999, // just inside domain -0.9999999999999999, // log1p domain boundary (> -1) - -0.9999999999999999, // just above -1 -1.0 + 1e-15, // very close to -1 -1.0000000000000002, // just below -1
🧹 Nitpick comments (1)
src/cmath/misc.rs (1)
47-59: Consider removing the unusedset_errno(0)call.The function calls
set_errno(0)but then checks overflow viar.is_infinite()rather than viaget_errno(). Per coding guidelines,set_errno(0)is valid before libm calls, but since errno isn't being checked afterward, this call has no effect.That said, this is a minor inconsistency and the logic is correct—overflow detection via
is_infinite()is reliable for hypot.♻️ Optional cleanup
fn c_abs_checked(z: Complex64) -> Result<f64> { if !z.re.is_finite() || !z.im.is_finite() { return Ok(c_abs_raw(z)); } - crate::err::set_errno(0); let r = m::hypot(z.re, z.im); if r.is_infinite() { Err(Error::ERANGE) } else { Ok(r) } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/cmath.rssrc/cmath/exponential.rssrc/cmath/misc.rssrc/cmath/trigonometric.rssrc/math.rssrc/math/exponential.rssrc/math/integer.rssrc/math/misc.rssrc/math/trigonometric.rssrc/test.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cmath/exponential.rs
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Files:
src/cmath.rssrc/math.rssrc/math/integer.rssrc/cmath/misc.rssrc/test.rssrc/math/exponential.rssrc/math/misc.rssrc/cmath/trigonometric.rssrc/math/trigonometric.rs
src/math.rs
📄 CodeRabbit inference engine (AGENTS.md)
Create missing helpers if CPython has a helper we don't have yet, such as math_1_fn for Rust function pointers or special case handlers for specific functions.
Files:
src/math.rs
src/math/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/math/**/*.rs: Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Files:
src/math/integer.rssrc/math/exponential.rssrc/math/misc.rssrc/math/trigonometric.rs
src/cmath/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/cmath/**/*.rs: For complex math (cmath module), copy special_type() enum and function, 7x7 special value tables (e.g., tanh_special_values), SPECIAL_VALUE macro usage, and complex-specific error handling from Modules/cmathmodule.c.
Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Files:
src/cmath/misc.rssrc/cmath/trigonometric.rs
src/**/*test*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*test*.rs: All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
Tests must verify both correct values for Ok results and correct error types (EDOM vs ERANGE) for Err results. Python ValueError maps to Error::EDOM and OverflowError maps to Error::ERANGE.
Files:
src/test.rs
src/math/exponential.rs
📄 CodeRabbit inference engine (AGENTS.md)
Copy CPython's explicit special case handling for IEEE specials (NaN, Inf, etc.) exactly, including logic for cases like NaN**0 = 1.
Files:
src/math/exponential.rs
src/cmath/trigonometric.rs
📄 CodeRabbit inference engine (AGENTS.md)
In cmath functions that use both sin and cos of the same angle (cosh, sinh, exp, rect), use m::sincos(x) which returns (sin, cos) tuple instead of calling sin and cos separately to match CPython's macOS __sincos_stret behavior.
Files:
src/cmath/trigonometric.rs
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.994Z
Learning: Applies to src/cmath/**/*.rs : Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.994Z
Learning: Applies to src/**/*test*.rs : All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
📚 Learning: 2026-01-10T13:40:01.994Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.994Z
Learning: Applies to src/cmath/**/*.rs : Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Applied to files:
src/cmath.rssrc/math.rssrc/math/integer.rssrc/cmath/misc.rssrc/math/exponential.rssrc/cmath/trigonometric.rssrc/math/trigonometric.rs
📚 Learning: 2026-01-10T13:40:01.994Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.994Z
Learning: Applies to src/math/**/*.rs : Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Applied to files:
src/cmath.rssrc/math.rssrc/cmath/misc.rssrc/math/exponential.rssrc/math/misc.rssrc/cmath/trigonometric.rssrc/math/trigonometric.rs
📚 Learning: 2026-01-10T13:40:01.994Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.994Z
Learning: Applies to src/**/*.rs : Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Applied to files:
src/cmath.rssrc/math.rssrc/math/integer.rssrc/cmath/misc.rssrc/test.rssrc/math/exponential.rssrc/math/misc.rssrc/cmath/trigonometric.rssrc/math/trigonometric.rs
📚 Learning: 2026-01-10T13:40:01.994Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.994Z
Learning: Applies to src/cmath/trigonometric.rs : In cmath functions that use both sin and cos of the same angle (cosh, sinh, exp, rect), use m::sincos(x) which returns (sin, cos) tuple instead of calling sin and cos separately to match CPython's macOS __sincos_stret behavior.
Applied to files:
src/cmath.rssrc/math.rssrc/cmath/misc.rssrc/math/exponential.rssrc/cmath/trigonometric.rssrc/math/trigonometric.rs
📚 Learning: 2026-01-10T13:40:01.994Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.994Z
Learning: Applies to src/math.rs : Create missing helpers if CPython has a helper we don't have yet, such as math_1_fn for Rust function pointers or special case handlers for specific functions.
Applied to files:
src/cmath.rssrc/math/integer.rssrc/cmath/misc.rssrc/test.rs
📚 Learning: 2026-01-10T13:40:01.994Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.994Z
Learning: Applies to src/math/**/*.rs : Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Applied to files:
src/cmath.rssrc/math.rssrc/math/integer.rssrc/cmath/misc.rssrc/math/exponential.rssrc/math/misc.rssrc/cmath/trigonometric.rssrc/math/trigonometric.rs
📚 Learning: 2026-01-10T13:40:01.994Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.994Z
Learning: Applies to src/**/*test*.rs : Tests must verify both correct values for Ok results and correct error types (EDOM vs ERANGE) for Err results. Python ValueError maps to Error::EDOM and OverflowError maps to Error::ERANGE.
Applied to files:
src/cmath.rssrc/math.rssrc/math/integer.rssrc/cmath/misc.rssrc/test.rssrc/math/exponential.rssrc/math/misc.rssrc/cmath/trigonometric.rssrc/math/trigonometric.rs
📚 Learning: 2026-01-10T13:40:01.994Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.994Z
Learning: Applies to src/math/exponential.rs : Copy CPython's explicit special case handling for IEEE specials (NaN, Inf, etc.) exactly, including logic for cases like NaN**0 = 1.
Applied to files:
src/cmath.rssrc/math.rssrc/cmath/misc.rssrc/test.rssrc/math/exponential.rssrc/math/misc.rs
📚 Learning: 2026-01-10T13:40:01.994Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.994Z
Learning: Applies to src/cmath/**/*.rs : For complex math (cmath module), copy special_type() enum and function, 7x7 special value tables (e.g., tanh_special_values), SPECIAL_VALUE macro usage, and complex-specific error handling from Modules/cmathmodule.c.
Applied to files:
src/cmath.rssrc/math.rssrc/cmath/misc.rssrc/cmath/trigonometric.rssrc/math/trigonometric.rs
📚 Learning: 2026-01-10T13:40:01.994Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.994Z
Learning: Applies to src/**/*test*.rs : All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
Applied to files:
src/math.rssrc/math/integer.rssrc/cmath/misc.rssrc/test.rssrc/math/exponential.rssrc/math/misc.rssrc/cmath/trigonometric.rssrc/math/trigonometric.rs
🧬 Code graph analysis (3)
src/cmath/misc.rs (3)
src/math/misc.rs (1)
fabs(30-32)src/math.rs (1)
hypot(109-132)src/err.rs (1)
set_errno(30-58)
src/math/exponential.rs (1)
src/cmath/exponential.rs (6)
edgetest_sqrt(331-337)test_sqrt(272-274)edgetest_log_n(349-355)test_log_n(278-280)edgetest_log10(358-364)test_log10(281-283)
src/cmath/trigonometric.rs (1)
src/math/trigonometric.rs (7)
test_sin(95-97)edgetest_cos(144-148)test_cos(98-100)edgetest_sinh(203-207)test_sinh(116-118)edgetest_atan(172-176)test_atan(110-112)
⏰ 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). (3)
- GitHub Check: Build and test (macos-latest)
- GitHub Check: Check exotic targets
- GitHub Check: Build and test (windows-latest)
🔇 Additional comments (19)
src/cmath/trigonometric.rs (1)
423-528: Edge-case loops updated cleanly.Iterating directly over the slice-based
EDGE_VALUESkeeps the tests consistent with the updated data structure. Looks good.src/math/trigonometric.rs (1)
135-227: EDGE_VALUES iteration updates look good.Direct iteration over the slice aligns with the new EDGE_VALUES representation and keeps test intent unchanged.
src/math/exponential.rs (1)
335-402: Edge-value test loops updated appropriately.Iteration over the slice-based
EDGE_VALUESis consistent and should preserve test coverage.src/math.rs (2)
115-129: Clearer hypot preprocessing with explicit NaN tracking.The explicit loop makes NaN handling and max tracking straightforward while preserving the vector_norm inputs.
209-272: Test updates align with slice-based EDGE_VALUES.The edge-test loops and constants checks are consistent with the new EDGE_VALUES layout.
src/math/misc.rs (3)
209-210: LGTM!The
EDGE_INTSconstant provides appropriate edge values for integer exponent testing inldexp, including boundary cases for typical exponent ranges.
312-366: LGTM!The test iteration patterns are correctly updated to work with the new slice-based
EDGE_VALUES. Usingfor &x in EDGE_VALUESis idiomatic for iterating over&[f64]while destructuring to get ownedf64values. The tests properly cover edge values as required by coding guidelines. Based on learnings, these tests usecrate::test::EDGE_VALUESwhich includes the required zeros, infinities, NaNs, subnormals, and boundary values.
574-582: LGTM!The
edgetest_fmaiteration pattern is correctly updated and tests the three-argument FMA function with appropriate edge value combinations.src/cmath/misc.rs (4)
31-45: LGTM!The
c_abs_rawfunction correctly implements C99 semantics for complex magnitude:
- Infinity in either component yields infinity (even if the other is NaN)
- NaN (without infinity) yields NaN
- Finite inputs delegate to
m::hypotThis aligns with CPython's
cmathspecial value handling. Based on learnings, this follows the pattern of copying special_type handling from CPython's cmathmodule.c.
63-67: LGTM!The
polarfunction now correctly usesc_abs_checkedfor magnitude computation with proper ERANGE error propagation on overflow.
122-125: LGTM!The
absfunction correctly delegates toc_abs_raw, providing C99-compliant magnitude computation for all special values.
212-219: LGTM!The test iteration patterns are correctly updated for the slice-based
EDGE_VALUES. The edge tests properly exercise phase, polar, and rect with comprehensive edge case coverage.Also applies to: 277-284, 324-331
src/test.rs (2)
1-2: LGTM!The
#![allow(clippy::excessive_precision)]is appropriate since the edge values intentionally use precise floating-point representations for testing boundary conditions.
106-127: LGTM!The
unwraphelper correctly maps Python exceptions to Rust error types per coding guidelines:
PyValueError→Error::EDOMPyOverflowError→Error::ERANGEThe change from
.ok().expect()to.expect()is a cleaner approach.src/math/integer.rs (5)
717-773: LGTM!The expanded
EDGE_I64provides excellent coverage for integer math functions:
- Table boundaries (127/128) test algorithm switching in comb/perm
- Factorial thresholds (12/13 for u32, 20/21 for u64, 170/171 for f64) test overflow boundaries
- Square root boundaries test the
isqrtalgorithm paths- Near-perfect squares (99, 101) test edge cases in sqrt approximation
This aligns well with the function implementations that use lookup tables and algorithm switching.
983-1010: LGTM!The
edgetest_gcdandedgetest_lcmiterations are correctly updated for slice-basedEDGE_I64. The additional boundary tests fori64::MAXandi64::MINcombinations provide good coverage.
1012-1030: LGTM!The
edgetest_isqrtcorrectly iterates overEDGE_I64and includes additional tests for perfect squares and their neighbors, which is important for testing the Newton's iteration convergence.
1032-1047: LGTM!The factorial edge test appropriately filters
EDGE_I64to the valid range(-10..=170)to avoid excessively long computations while still covering error cases (negative values) and the f64 overflow boundary (170/171).
1049-1084: LGTM!The comb and perm edge tests appropriately filter values to reasonable ranges to avoid combinatorial explosion while still covering algorithm switching boundaries and error cases. The large value tests (e.g.,
comb(1000, 500),perm(1000, Some(3))) verify bigint handling.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary by CodeRabbit
Tests
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.