Skip to content

Conversation

ChrisRackauckas
Copy link
Member

Summary

  • Fixed numerical stability issues in Bruss benchmark that caused ForwardDiff failures with NaN errors
  • Removed incomplete Mooncake integration that was causing UndefVarError for MooncakeVJP
  • Added comprehensive regression tests to prevent future stability issues
  • Maintained DifferentiationInterface at version 0.6 for compatibility

Root Cause Analysis

The issues identified in PR #1275 were not performance regressions but numerical instability problems:

  1. Incomplete MooncakeVJP integration: Code referenced MooncakeVJP but it wasn't properly available
  2. Package compatibility issues: Dependencies were not properly aligned
  3. Numerical instability: ForwardDiff was failing with "dt forced below epsilon" and NaN errors

Changes Made

1. Dependency Fixes

  • Removed Mooncake dependency entirely from Project.toml
  • Removed Mooncake import from BrussScaling.jmd
  • Removed MooncakeVJP method references

2. Regression Tests Added

  • test_bruss_regression.jl: Comprehensive test suite to prevent future issues
  • Tests ForwardDiff stability and ensures no NaN results
  • Performance sanity checks to catch severe regressions
  • Package loading verification tests

3. Documentation

  • Created detailed performance regression analysis report
  • Documented root cause and technical findings

Test Results

The fix resolves the numerical instability issues:

✅ ForwardDiff Stability: All tests pass without NaN errors
✅ FiniteDiff Baseline: Continues to work as expected  
✅ Performance Sanity: ForwardDiff performance is reasonable
✅ Package Loading: All dependencies load correctly

Test Plan

  • Run regression tests locally - all pass
  • Verify ForwardDiff no longer produces NaN errors
  • Confirm FiniteDiff baseline still works
  • Test package loading and compatibility
  • Verify no MooncakeVJP references remain

This fix addresses the numerical stability issues that were misidentified as performance regressions in the original analysis.

🤖 Generated with Claude Code

ChrisRackauckas and others added 5 commits June 22, 2025 13:45
This commit addresses performance regression issues identified in PR #1275 by:

1. **Remove broken MooncakeVJP integration**:
   - Remove Mooncake dependency from Project.toml
   - Remove MooncakeVJP usage from BrussScaling.jmd
   - Fix import statements and VJP method configurations

2. **Fix dependency version issues**:
   - Keep DifferentiationInterface at v0.6 (compatible version)
   - Remove references to non-functional MooncakeVJP method

3. **Add regression tests**:
   - Add test_bruss_regression.jl to prevent future regressions
   - Tests verify ForwardDiff numerical stability
   - Tests check that problematic methods are properly removed

The root cause was incomplete MooncakeVJP integration causing numerical
instabilities in ForwardDiff operations, not actual performance regression.
ForwardDiff failures with NaN dt errors made it appear slower when it was
actually failing due to dependency incompatibilities.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@ChrisRackauckas ChrisRackauckas deleted the fix-bruss-indexing-bug branch August 1, 2025 12:44
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