Skip to content

Fix mul! for uninitialized matrices and add interface tests#17

Merged
ChrisRackauckas merged 2 commits into
SciML:mainfrom
ChrisRackauckas-Claude:interface-check-20260107-133126
Jan 11, 2026
Merged

Fix mul! for uninitialized matrices and add interface tests#17
ChrisRackauckas merged 2 commits into
SciML:mainfrom
ChrisRackauckas-Claude:interface-check-20260107-133126

Conversation

@ChrisRackauckas-Claude
Copy link
Copy Markdown

Summary

  • Fix mul! methods to handle uninitialized output matrices by checking if b is zero and using fill!(C, zero(T)) instead of C.*=b
  • Add comprehensive interface compatibility tests for BigFloat support
  • Add tests for AbstractArray interface compliance

Problem

The * operator for SparseBandedMatrix was failing when used with BigFloat element types. This was because:

  1. The * operator creates an uninitialized output matrix using similar()
  2. It then calls mul!(C, A, B, true, false) where the false (b=0) argument should zero out C
  3. The C.*=b pattern fails when C contains undefined references (like #undef in uninitialized BigFloat arrays)

Solution

Replace C.*=b with:

if iszero(b)
    fill!(C, zero(T))
else
    C .*= b
end

This ensures proper initialization of the output matrix regardless of element type.

Interface Testing

Added comprehensive interface tests for:

  • BigFloat support (constructor, indexing, setdiagonal!, mul!, * operator)
  • AbstractArray interface compliance (size, length, eltype, axes, iteration, Matrix conversion)

Note: ComplexF64 is not fully supported because fma() is not defined for complex numbers. This is a known limitation of the current implementation.

Test plan

  • All existing tests pass
  • New interface tests pass
  • BigFloat * operator now works correctly

cc @ChrisRackauckas

🤖 Generated with Claude Code

@ChrisRackauckas-Claude ChrisRackauckas-Claude force-pushed the interface-check-20260107-133126 branch 2 times, most recently from a3f47fa to ef1c2ce Compare January 11, 2026 15:08
@ChrisRackauckas-Claude
Copy link
Copy Markdown
Author

Rebase and CI Fix Summary

The PR has been rebased onto the latest main branch and the following fixes were made:

Changes Made

  1. Rebased onto main - Resolved merge conflicts in src/SparseBandedMatrices.jl
  2. Fixed typo in comment - Changed # C = Cb + aBA to # C = C*b + a*B*A to pass spell check
  3. Fixed docs/Project.toml UUID - Updated SparseBandedMatrices UUID from b42f87ec-cfbe-4098-ba17-1affa37bb2d8 to bd59d7e1-4699-4102-944e-d05209cb92aa

CI Status

Passing:

  • test - Main test suite passes
  • Spell Check with Typos - Fixed
  • runic - Code formatting passes

Pre-existing failures (also failing on main):

  • build (Documentation) - The docs/make.jl tries to copy a non-existent Manifest.toml file. This is a pre-existing issue on main.
  • Tests (pre) - JET.jl is not compatible with Julia 1.13.0-alpha2. This is a pre-existing issue on main.

The PR-specific issues have been resolved. The remaining failures are infrastructure issues that also affect the main branch.

@ChrisRackauckas-Claude ChrisRackauckas-Claude force-pushed the interface-check-20260107-133126 branch 2 times, most recently from 34b7dff to 22f25f1 Compare January 11, 2026 15:54
@ChrisRackauckas-Claude
Copy link
Copy Markdown
Author

Update: CI Fixes Applied

All requested fixes have been implemented:

Changes Made

  1. Fixed docs/make.jl - Added mkpath to create the assets directory before copying manifest files
  2. Deleted test/Project.toml - Moved test dependencies to main Project.toml [extras] and [targets] sections
  3. Added GROUP-based JET test skipping - JET tests now only run when GROUP != "core"
  4. Updated Tests.yml workflow - Pre-release Julia now runs with GROUP="Core" to skip JET tests

Current CI Status

Check Status Notes
build (Documentation) ✅ PASS Fixed
test ✅ PASS Main test suite
Spell Check ✅ PASS
runic ✅ PASS Formatting
Tests (1) ✅ PASS Julia 1.x with JET
Tests (lts) ✅ PASS Julia LTS with JET
Tests (pre) ❌ FAIL Julia 1.13-alpha2 (JET skipped, but other pre-release issues)

The Tests (pre) failure appears to be a Julia 1.13.0-alpha2 compatibility issue unrelated to JET (which is now correctly skipped). This is a pre-release Julia version and such failures are expected.

- Fix mul! methods to handle uninitialized output matrices by checking
  if b is zero and using fill!(C, zero(T)) instead of C.*=b, which
  fails on matrices with undefined references (e.g., BigFloat matrices
  created by similar())
- Add comprehensive interface compatibility tests for BigFloat support
- Add tests for AbstractArray interface compliance
- This enables the * operator to work correctly with BigFloat matrices

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ChrisRackauckas-Claude ChrisRackauckas-Claude force-pushed the interface-check-20260107-133126 branch from 22f25f1 to 38eb09b Compare January 11, 2026 16:02
- Create test/Project.toml with core test dependencies
- Create test/nopre/ subdirectory with JET dependency
- Update runtests.jl to use GROUP-based test routing
- Update CI workflow to exclude pre-release Julia from nopre tests
- Clean up main Project.toml by removing unused extras/targets

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ChrisRackauckas ChrisRackauckas merged commit 22d86a3 into SciML:main Jan 11, 2026
9 checks 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.

3 participants