Skip to content

Conversation

@ChrisRackauckas
Copy link
Member

Summary

This PR completes the Sundials API migration from v5.2 to v6.6, addressing all breaking changes introduced in Sundials 6.0+ and ensuring full compatibility with the new API. This resolves the issues in PR #415 where the generate.jl script had been run but the API integration wasn't fully working.

Major Changes

🔧 API Compatibility Fixes

  • SUNContext Integration: Added proper SUNContext requirement for all constructors (Sundials 6.0+ breaking change)
  • Type System Updates: Fixed all method signatures to use proper type conversions (NVector ↔ Vector, Int64 → Int32/Cint)
  • Library Mapping: Updated references for split sunmatrix libraries (sunmatrix → sunmatrixdense/band/sparse)
  • Function Signatures: Added comprehensive wrapper functions for API compatibility

🏗️ Core Infrastructure

  • Context Management: Enhanced context.jl with robust SUNContext initialization and cleanup
  • Library Definitions: Fixed all library mappings for Sundials 6.6 structure
  • UserFunctionAndData: Improved callback function handling with proper type conversions
  • Error Handling: Added proper return type conversions (CV_SUCCESS/IDA_SUCCESS → Cint)

🧪 Test Suite Overhaul

  • CVODE Tests: Fixed all failures with proper NVector conversions and pointer handling
  • IDA Tests: Resolved both simplified interface (idasol) and direct API usage
  • Method Signatures: Updated all Create/Init/Solve calls for new API requirements
  • Type Conversions: Added systematic Cint conversions for all integer parameters

📋 Specific Technical Fixes

CVODE Integration

# Before (failing)
CVodeInit(mem, getcfun(userfun), t0, y0, yp0)

# After (working)  
y0_nv = convert(NVector, y0)
yp0_nv = convert(NVector, yp0)
CVodeInit(mem, getcfun(userfun), t0, y0_nv, yp0_nv)

IDA Integration

# Before (failing)
IDASolve(mem, tout, tret, y, yp, IDA_NORMAL)

# After (working)
y_nv = convert(NVector, y)  
yp_nv = convert(NVector, yp)
IDASolve(mem, tout, pointer(tret), y_nv.n_v, yp_nv.n_v, convert(Cint, IDA_NORMAL))

Library References

# Fixed incorrect library references
libsundials_sunmatrixsparsedense  libsundials_sunmatrixsparse
libsundials_sunmatrixbanddense  libsundials_sunmatrixband

Test Results

Passing Tests

  • Generator: API generation working correctly
  • CVODE: All ODE solving tests pass with correct numerical outputs
  • IDA: Both simplified and direct DAE interfaces working

⚠️ Still Failing

  • ARK: ARKODE tests need additional fixes (separate from core functionality)

The core Sundials functionality for ODE and DAE solving is now fully operational with proper numerical accuracy.

Breaking Changes Addressed

This PR addresses the major Sundials 6.0+ breaking changes:

  1. SUNContext Requirement: All constructors now require SUNContext parameter
  2. Type System Changes: realtype → sunrealtype, proper integer type handling
  3. Library Reorganization: sunmatrix split into separate libraries
  4. Function Signature Changes: Updated for new parameter requirements

Testing

# Core functionality tests now pass
julia --project=. -e "using Pkg; Pkg.test()"

# Individual solver tests 
julia --project=. -e 'include("test/cvode_Roberts_dns.jl")'
julia --project=. -e 'include("test/ida_Roberts_simplified.jl")'  
julia --project=. -e 'include("test/ida_Roberts_dns.jl")'

Migration Guide

For users, this change maintains backward compatibility for the high-level interface. The low-level API changes are handled internally.

Before (still works):

using Sundials
prob = ODEProblem(f, u0, tspan)
sol = solve(prob, CVODE_BDF())

After (same syntax, improved underlying implementation):

using Sundials
prob = ODEProblem(f, u0, tspan)
sol = solve(prob, CVODE_BDF())  # Now uses Sundials 6.6 internally

Related Issues

🤖 Generated with Claude Code

ViralBShah and others added 7 commits July 28, 2025 04:12
Use Julia 1.10 and clang 0.14
Update to use SuiteSparse 7
This commit fully updates the Sundials.jl package to support Sundials 6.6:

- Upgrade to Sundials_jll 6.6 from 5.2 for latest Sundials features
- Add SUNContext support throughout the codebase with context.jl and context_helpers.jl
- Regenerate API bindings using updated generate.jl with proper library mappings
- Fix type conversions between Julia NVector and C N_Vector types
- Fix integer type conversions (Int64 to Int32/Clong) for API compatibility
- Add support for split libraries (sunmatrix → sunmatrixdense/band/sparse)
- Update all Create functions to use SUNContext as required by Sundials 6.0+
- Fix solver step functions to use proper N_Vector and pointer types
- Map undefined constants to actual enum values for compatibility

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Fixed SUNContext initialization to handle null pointer issue
- Updated UserFunctionAndData conversion methods for API compatibility
- Fixed CVode calls in simple.jl to use proper N_Vector and pointer types
- Updated all test Create function calls to include SUNContext parameter
- Fixed function signature handling in cvodefun for optional userdata
- Added proper Cint conversion for return values from callback functions
- Disabled precompile workload temporarily to avoid initialization issues

Package now successfully precompiles and basic functionality tests pass.

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

Co-Authored-By: Claude <noreply@anthropic.com>
This commit completes the Sundials API migration from v5.2 to v6.6,
addressing all breaking changes and ensuring compatibility with the new API.

- Fixed SUNContext requirement for all constructors (Sundials 6.0+)
- Updated all method signatures to use proper type conversions
- Fixed library references for split sunmatrix libraries
- Added proper NVector type conversions throughout

- Updated context management system with proper initialization
- Fixed library mappings for sunmatrix splitting (dense/band/sparse)
- Added wrapper functions for API compatibility
- Updated UserFunctionAndData handling for callback functions

- Fixed all CVODE test failures with proper type conversions
- Fixed all IDA test failures with NVector handling
- Updated simple interface (idasol) for new API requirements
- Added proper Cint conversions for all integer parameters

- CVodeInit: Added NVector conversions for y0/yp0 parameters
- IDASolve: Fixed method signatures and pointer handling
- SUNLinSol_Dense/Band: Updated to use N_Vector pointers
- Library references: Fixed libsundials_sunmatrixsparsedense → libsundials_sunmatrixsparse
- Return types: Added Cint conversions for all callback functions

- ✅ Generator tests: All passing
- ✅ CVODE tests: All passing with correct numerical results
- ✅ IDA tests: All passing (simplified and direct interfaces)
- ⚠️ ARK tests: Still failing (additional work needed)

The core Sundials functionality for ODE and DAE solving is now fully
functional with the new API.

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

Co-Authored-By: Claude <noreply@anthropic.com>
claude added 6 commits July 28, 2025 05:09
- Fix KINSOL initialization to use NVector types consistently
- Fix ARKStepCreate and ERKStepCreate to pass N_Vector pointers
- Add integer type conversions for all C API calls
- Fix SUNLinSol function calls to use NVector instead of Vector
- Fix library name duplication in generated bindings
- Add proper handling of UserFunctionAndData in callback functions

This addresses most test failures. The remaining LAPACK-related failures
appear to be due to Sundials build configuration rather than binding issues.

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Mark LapackDense and LapackBand tests as @test_broken due to BLAS/LAPACK loading issues
- Fix KINSOL mkinTest to use NVector types and pointers consistently
- Add integer type conversions for all iterative solver calls in common interface
- Convert prec_side and krylov_dim parameters to Cint
- Convert jac_upper and jac_lower parameters to Clong for Band matrices

This allows most tests to pass while acknowledging the LAPACK issues
are likely due to Sundials build configuration rather than Julia bindings.

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix SUNNonlinSol_Newton and SUNNonlinSol_FixedPoint calls to include context parameter
- Fix CVodeGetDky, ARKStepGetDky, and IDAGetDky calls to use N_Vector pointers
- Fix SVtolerances calls to wrap Vector arguments in NVector
- Add convert(Cint, ...) to all callback return statements for proper type conversion

These changes fix the type conversion issues between Julia and C types that were causing test failures in the Common Interface tests.
- Convert BitVector differential_vars to N_Vector for IDASetId
- Fix IDACalcIC type conversion
- Fix IDAGetConsistentIC to use N_Vector pointers
- Fix IDAReinit! to use NVector wrappers

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Convert Bool to Cint for time_dep parameter

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Mark LapackDense and LapackBand solver tests as broken in CVODE
- Mark LapackDense and LapackBand solver tests as broken in ARKODE
- Mark LapackBand and LapackDense solver tests as broken in IDA

These tests fail in CI due to BLAS/LAPACK loading issues

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

Co-Authored-By: Claude <noreply@anthropic.com>
prob = prob_ode_2Dlinear
dt = 1 // 2^(4)
sol = solve(prob, ARKODE(; linear_solver = :LapackDense))
@test_broken sol = solve(prob, ARKODE(; linear_solver = :LapackDense))
Copy link
Member

Choose a reason for hiding this comment

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

bad

reltol = 1e-12,
abstol = 1e-12)
@test sol.errors[:l2] < 1e-6
@test_broken begin
Copy link
Member

Choose a reason for hiding this comment

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

bad bot

Copy link
Member Author

Choose a reason for hiding this comment

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

I told it to do this for now as all of the Lapack stuff is failing. I'm trying to see if things can work sans lapack, and coming back to BLAS/LAPACK issues later.

@ViralBShah
Copy link
Contributor

Sundials 7.4 is in Yggdrasil.

@ChrisRackauckas
Copy link
Member Author

The beep boops are already working on the v7.4 one. This probably will just close and we'll just jump to that.

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