Skip to content

Refactor Fixed type to use runtime scale#18

Merged
raphaelDkhn merged 2 commits into
gizatechxyz:masterfrom
MahmoudMohajer:dyn-fixed
Oct 7, 2025
Merged

Refactor Fixed type to use runtime scale#18
raphaelDkhn merged 2 commits into
gizatechxyz:masterfrom
MahmoudMohajer:dyn-fixed

Conversation

@MahmoudMohajer
Copy link
Copy Markdown
Contributor

Refactor Fixed type to use runtime scale and simplify evaluation tests. Update Fixed struct to store scale as a field, modify related methods, and adjust tests for new Fixed implementation.

Pull Request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: N/A

Previously we used const generics to set the scale of a Fixed number at compile time. This approach required specifying the scale as a type parameter (Fixed<SCALE>) and made it impossible to have dynamic scale values at runtime. This limitation prevented integration with LuminAir's dynamic fixed-point scale requirements.

What is the new behavior?

  • Runtime scale support: The Fixed struct now stores scale as a runtime field instead of a const generic parameter
  • Dynamic scale selection: Scale can now be specified at runtime using Fixed::from_f64(value, scale) or Fixed::new(value, scale)
  • Scale validation: All arithmetic operations now validate that operands have matching scales
  • Backward compatibility: Added convenience methods like from_f64_8(), from_f64_12(), etc. for common scales
  • Enhanced API: Added methods for scale management (scale(), value(), convert_to(), zero())
  • Comprehensive testing: Restored all previously removed comprehensive tests with 1000+ random test cases

Key Changes:

  • Fixed<const SCALE: u32>(i64)Fixed { value: i64, scale: u32 }
  • from_f64(value)from_f64(value, scale)
  • Added scale mismatch assertions in arithmetic operations
  • Updated all evaluation tests to use runtime scale API

Does this introduce a breaking change?

  • Yes
  • No

Breaking Changes:

  • API Changes: All Fixed::from_f64() calls now require a scale parameter
  • Type Changes: Fixed<SCALE> becomes Fixed (no const generic)
  • Field Access: .0 field access becomes .value field access

Migration Path:

// Before
let a = Fixed::<15>::from_f64(1.5);
let b = Fixed::<15>::from_f64(2.0);

// After  
let a = Fixed::from_f64(1.5, 15);
let b = Fixed::from_f64(2.0, 15);

// Or use convenience methods
let a = Fixed::from_f64_15(1.5); // if you add this method
let b = Fixed::from_f64_15(2.0);

Testing

  • ✅ All 19 tests pass (up from 11)
  • ✅ Comprehensive random testing (1000+ iterations per operation)
  • ✅ Edge case testing (negative numbers, large values, small values)
  • ✅ Scale conversion and mismatch testing
  • ✅ Evaluation constraint testing

Other information

This refactoring enables dynamic scale selection which is essential for LuminAir integration. The change maintains all existing functionality while providing the flexibility needed for runtime scale determination.

Files Changed:

  • src/lib.rs - Main Fixed type refactoring and comprehensive test restoration
  • src/eval.rs - Updated evaluation tests to use runtime scale API

Test Coverage:

  • Basic operations, multiplication, scale conversion
  • Negative number handling
  • Random testing (1000 iterations each for add/sub/mul)
  • Reciprocal and square root operations
  • Remainder and division operations
  • Scale mismatch validation
  • Evaluation constraint testing

…s. Update Fixed struct to store scale as a field, modify related methods, and adjust tests for new Fixed implementation.
… subtraction, multiplication, division, remainder, and square root. Utilize random number generation for extensive coverage and edge case handling.
Copy link
Copy Markdown
Contributor

@raphaelDkhn raphaelDkhn left a comment

Choose a reason for hiding this comment

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

LGTM!

@raphaelDkhn raphaelDkhn merged commit c498194 into gizatechxyz:master Oct 7, 2025
1 check 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.

2 participants