Skip to content

Conversation

ChrisRackauckas-Claude
Copy link
Contributor

Summary

This PR enhances the logging system introduced in #622 by adding detailed BLAS/LAPACK return code interpretation and extended logging information.

Changes

1. BLAS/LAPACK Return Code Interpretation

  • Added interpret_blas_code() function that provides human-readable explanations for all BLAS/LAPACK info codes
  • Categorizes errors (singular_matrix, not_positive_definite, convergence_failure, etc.)
  • Operation-specific interpretations for different LAPACK functions (getrf, potrf, geqrf, gesdd, etc.)

2. Extended Logging Information

  • Matrix properties (size, type, condition number when feasible)
  • Memory usage estimates for BLAS operations
  • Performance timing metrics with opt-in verbosity control
  • Additional context for debugging numerical issues

3. New Verbosity Controls

Added five new verbosity settings to the LinearVerbosity system:

  • blas_errors: Controls BLAS/LAPACK error messages (default: Warn)
  • blas_info: Controls informational messages (default: None)
  • blas_success: Controls success messages (default: None)
  • blas_invalid_args: Controls invalid argument errors (default: Error)
  • blas_timing: Controls performance timing (default: None)

4. Integration Example (BLISLUFactorization)

Demonstrated integration with the BLIS extension to show how the new logging can be applied to existing solvers.

5. Documentation

  • Updated verbosity documentation with all new BLAS options
  • Added comprehensive section on BLAS/LAPACK return codes
  • Included examples demonstrating the enhanced logging capabilities

6. Tests

  • Added comprehensive test suite for BLAS return code interpretation
  • Tests for different error categories
  • Verbosity integration tests

Example Output

When a singular matrix is encountered:

BLAS/LAPACK dgetrf: Matrix is singular
  Details: U(2,2) is exactly zero. The factorization has been completed, but U is singular
  Return code (info): 2
  matrix_size: (2, 2)
  element_type: Float64
  condition_number: Inf
  memory_usage_MB: 0.0

Test Plan

  • Tests pass locally
  • Documentation updated
  • Backwards compatible (all new features are opt-in)

🤖 Generated with Claude Code

claude added 5 commits August 18, 2025 19:40
This commit builds upon PR SciML#622's verbosity system by adding:

1. Detailed BLAS/LAPACK return code interpretation
   - Human-readable explanations for all BLAS/LAPACK info codes
   - Categorized errors (singular_matrix, not_positive_definite, etc.)
   - Operation-specific interpretations for getrf, potrf, geqrf, etc.

2. Extended logging information for BLAS operations
   - Matrix properties (size, type, condition number)
   - Memory usage estimates
   - Performance timing metrics
   - Contextual information for debugging

3. New verbosity controls
   - blas_errors: Controls BLAS/LAPACK error messages (default: Warn)
   - blas_info: Controls informational messages (default: None)
   - blas_success: Controls success messages (default: None)
   - blas_invalid_args: Controls invalid argument errors (default: Error)
   - blas_timing: Controls performance timing (default: None)

4. Integration with BLISLUFactorization
   - Added detailed logging to the BLIS extension
   - Includes timing and error interpretation

5. Comprehensive documentation
   - Updated verbosity documentation with new BLAS options
   - Added section on BLAS/LAPACK return codes
   - Examples demonstrating enhanced logging capabilities

6. Tests
   - Added test suite for BLAS return code interpretation
   - Tests for different error categories
   - Verbosity integration tests

This enhancement makes debugging numerical issues much easier by providing
clear, actionable information when BLAS/LAPACK operations encounter problems.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Based on review feedback:

1. Removed all BLAS timing functionality
   - Removed time_blas_operation function
   - Removed blas_timing verbosity setting
   - Removed timing from documentation and tests

2. Made get_blas_operation_info conditional on logging
   - Only calls get_blas_operation_info when logging is actually enabled
   - Avoids unnecessary computation when logging is disabled

3. Made condition number computation optional
   - Added compute_condition parameter (default: false)
   - Only computes condition number when explicitly requested
   - Avoids expensive computation for large matrices

4. Updated BLISLUFactorization extension
   - Removed timing calls
   - Only gathers operation info when needed for logging
   - More efficient checking of verbosity levels

5. Updated documentation
   - Removed all references to timing functionality
   - Documented optional condition number computation

6. Updated tests
   - Removed timing tests
   - Added tests for optional condition number computation

These changes make the logging more efficient by avoiding
unnecessary computations when logging is disabled.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Based on review feedback:

1. Replaced direct logging with @SciMLMessage macro
   - Updated log_blas_info to use @SciMLMessage with proper syntax
   - Structured log messages with appropriate categories and groups
   - Removed direct @info/@warn calls

2. Added condition_number as a separate verbosity setting
   - Added condition_number to LinearNumericalVerbosity struct
   - Condition number computation now controlled by verbosity setting
   - Default is Verbosity.None() to avoid expensive computation

3. Fixed verbosity checks
   - Use verbosity_to_int() for proper verbosity level checking
   - Ensures correct comparison with Verbosity.None()

4. Updated BLISLUFactorization extension
   - Uses @SciMLMessage for success messages
   - Proper verbosity checks before computing operation info
   - Only calls get_blas_operation_info when logging is enabled

5. Updated documentation
   - Added condition_number verbosity setting documentation
   - Clarified that condition number is controlled by verbosity

6. Updated tests
   - Tests now use verbosity parameter for condition number control
   - Verifies condition number is not computed when disabled

These changes ensure efficient logging with proper use of the
SciMLLogging infrastructure and make condition number computation
a controllable option.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Replaced the last remaining @info call in check_and_log_lapack_result
with @SciMLMessage macro for consistency with the SciMLLogging system.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Replace equality checks (!=) with identity checks (!==) for
Verbosity.None() comparisons to enable compile-time optimization.
This ensures logging code is completely compiled out when disabled,
providing zero-cost abstraction for the verbosity system.

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

Co-Authored-By: Claude <noreply@anthropic.com>
@ChrisRackauckas
Copy link
Member

@jClugstor how do we set these up with verbosity levels?

@jClugstor
Copy link
Member

You're talking about distinct levels like, level 1 should give some info, level 2 should give some more etc. ?

We could add Verbosity types like Verbosity.Low(), Verbosity.Medium(), Verbosity.High() to SciMLLogging, and make it so that the constructors for LinearVerbosity take them and look at a dictionary to see what each setting should be set to.

@ChrisRackauckas
Copy link
Member

Yeah that would be a good thing to do, with some definition of what those mean.

Copy link
Member

@jClugstor jClugstor left a comment

Choose a reason for hiding this comment

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

Seems like everything is consistent with how SciMLLogging and the LinearVerbosity stuff works.

prob = LinearProblem(A, b)

# This should fail due to singularity but not throw
sol = solve(prob, LUFactorization(); verbose=verbose)
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't actually test that a message was logged at the correct level.
It might be good to have sol = @test_logs (:warn, "BLAS/LAPACK dgetrf: Matrix is singular Details: U(2,2) is exactly zero. The factorization has been completed, but U is singular" match_mode=:any solve(prob, LUFactorization(); verbose=verbose) with whatever the actual log message ends up being, so that the tests make sure the correct things end up in the logs.

Copy link
Member

Choose a reason for hiding this comment

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

give the suggestion?

b_good = [1.0, 1.0]
prob_good = LinearProblem(A_good, b_good)

sol_good = solve(prob_good, LUFactorization(); verbose=verbose_all)
Copy link
Member

Choose a reason for hiding this comment

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

Same thing as above here, @test_log (:info ...

prob = LinearProblem(A, b)

# This should fail due to singularity but not throw
sol = solve(prob, LUFactorization(); verbose=verbose)
Copy link
Member

@jClugstor jClugstor Aug 21, 2025

Choose a reason for hiding this comment

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

Suggested change
sol = solve(prob, LUFactorization(); verbose=verbose)
sol = @test_logs (:warn, r"Matrix is singular") match_mode=:any solve(prob, LUFactorization(); verbose=verbose)

b_good = [1.0, 1.0]
prob_good = LinearProblem(A_good, b_good)

sol_good = solve(prob_good, LUFactorization(); verbose=verbose_all)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sol_good = solve(prob_good, LUFactorization(); verbose=verbose_all)
sol_good = @test_logs (:info, r"Operation completed successfully") match_mode=:any solve(prob, LUFactorization(); verbose=verbose)

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.

4 participants