Skip to content

Implement num_traits::FloatConst for Df64#19

Open
sakuraba07 wants to merge 3 commits into
tuwien-cms:mainlinefrom
sakuraba07:add-FloatConst
Open

Implement num_traits::FloatConst for Df64#19
sakuraba07 wants to merge 3 commits into
tuwien-cms:mainlinefrom
sakuraba07:add-FloatConst

Conversation

@sakuraba07

Copy link
Copy Markdown
Contributor

Closes #18

Summary

This PR implements the num_traits::FloatConst trait for Df64, providing standard mathematical constants in double-double precision.

Changes

New constants in consts.rs

  • SQRT_2 - Square root of 2
  • FRAC_1_SQRT_2 - Reciprocal of square root of 2 (1/√2)
  • LOG10_2 - Logarithm base-10 of 2
  • LOG2_10 - Logarithm base-2 of 10

FloatConst implementation in traits.rs

  • All 16 required methods: E, PI, FRAC_1_PI, FRAC_2_PI, FRAC_PI_2, FRAC_PI_3, FRAC_PI_4, FRAC_PI_6, FRAC_PI_8, FRAC_1_SQRT_2, FRAC_2_SQRT_PI, LN_2, LN_10, LOG2_E, LOG10_E, SQRT_2
  • All 3 provided methods with optimized implementations: TAU, LOG10_2, LOG2_10

Tests

  • Added accuracy tests for new constants in consts.rs

- Add SQRT_2, FRAC_1_SQRT_2, LOG10_2, LOG2_10 constants to consts.rs
- Implement all 16 required methods and 3 provided methods of FloatConst trait
- Add accuracy tests for new constants in consts.rs

@shinaoka shinaoka left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Comments

1. Recommend testing SQRT_2 / FRAC_1_SQRT_2 against computed values

The existing tests validate other constants against their computing functions (e.g., exp::exp(ONE) == EULER_E, circular::sin(PI_HALF) == ONE). For consistency and stronger validation, consider adding:

assert_ulps_eq!(SQRT_2, roots::sqrt(Df64::from(2.0)));
assert_ulps_eq!(FRAC_1_SQRT_2, roots::inv_sqrt(Df64::from(2.0)));

The current algebraic identity test (SQRT_2 * SQRT_2 == 2) alone may not catch a transcription error in hi/lo that degrades precision.

2. Recommend independent verification tests for LOG10_2 / LOG2_10

Currently only the product LOG10_2 * LOG2_10 == 1 is tested. Testing each constant independently would be more robust, since the product test cannot detect errors if both constants are off in the same direction:

assert_ulps_eq!(LOG10_2, LOG10_E * LN_2);  // log10(2) = log10(e) * ln(2)
assert_ulps_eq!(LOG2_10, LOG2_E * LN_10);  // log2(10) = log2(e) * ln(10)

3. (Nice-to-have) Test FloatConst trait methods directly

The tests only validate the raw constants in consts.rs. A brief sanity check through the trait interface would confirm the mapping is correct:

use num_traits::FloatConst;
assert_eq!(Df64::PI(), consts::PI);
assert_eq!(Df64::SQRT_2(), consts::SQRT_2);

@sakuraba07

Copy link
Copy Markdown
Contributor Author

Thank you for the review. I have updated the tests to address your suggestions:

  • Robust validation for SQRT_2 and FRAC_1_SQRT_2: These are now validated against computed values from arith::sqrt_q and roots::inv_sqrt.
  • Independent verification for LOG10_2 and LOG2_10: Replaced the product test with independent identity checks (log10(2) = log10(e) * ln(2) and log2(10) = log2(e) * ln(10)).
  • Direct FloatConst trait testing: Added a new test test_float_const in src/traits.rs to verify that all trait methods are correctly implemented.

All tests passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement num_traits::FloatConst

2 participants