Skip to content

Add linear system solver (Ax = b) using LAPACK dgesv#51

Merged
Quafadas merged 6 commits into
mainfrom
copilot/add-ax-equals-b-solver
Nov 25, 2025
Merged

Add linear system solver (Ax = b) using LAPACK dgesv#51
Quafadas merged 6 commits into
mainfrom
copilot/add-ax-equals-b-solver

Conversation

Copilot AI commented Nov 24, 2025

Copy link
Copy Markdown
Contributor

Implementation Plan for Linear System Solver (Ax = b)

This PR implements a linear system solver using LAPACK's dgesv routine, following the pattern established in the repository for other LAPACK-based operations (eigenvalues, SVD, Cholesky).

Tasks:

  • Understand repository structure and existing patterns
  • Review LAPACK facade usage in eig.scala, svd.scala, cholesky.scala
  • Understand test patterns from eigenvalue.test.scala
  • Implement JVM solver in vecxt/src-jvm/solve.scala using LAPACK dgesv
  • Create placeholder implementation in vecxt/src-js-native/solve.scala
  • Add solve export to vecxt/src/all.scala
  • Create comprehensive test suite in vecxt/test/src-jvm/solve.test.scala (25 tests)
  • Verify tests pass on JVM (all 25 tests passing)
  • Format code with scalafmt
  • Review test coverage (Score: 10/10 - exceeds 8+ target)
  • Run code review (✅ No issues found)
  • Run security scan with codeql_checker (✅ No vulnerabilities)
  • Address code review feedback - refactor dimension checks
  • Address code review feedback - remove unnecessary array copy

Recent Changes (per code review):

  • Latest: Removed unnecessary array copy in Array[Double] overload - now returns xMatrix.raw directly for better performance
  • Refactored dimension checking to use inline bounds check pattern
  • Moved dimension validation from require statement to inline check
  • Follows same pattern as nonEmptyMatCheck and squareMatCheck
  • Simplified bounds checking logic while maintaining safety
  • All tests continue to pass

Implementation Summary:

Solver Features:

  • Uses LAPACK dgesv for solving Ax = b on JVM
  • Two API variants: solve(A, b: Array[Double]) and solve(A, b: Matrix[Double])
  • Proper bounds checking with BoundsCheck typeclass
  • Comprehensive error handling for singular matrices, NaN inputs, dimension mismatches
  • Original matrices preserved (no mutation)
  • Optimized to avoid unnecessary copies
  • Placeholder implementations for JS/Native platforms

Test Coverage: 10/10 (25 comprehensive tests) - all passing

Security Summary:

No security vulnerabilities detected. The implementation:

  • Properly validates inputs (dimension checking, NaN detection)
  • Handles errors appropriately (singular matrices, invalid arguments)
  • Uses established LAPACK routines with proper error checking
  • No buffer overflows or memory safety issues
Original prompt

This section details on the original issue you should resolve

<issue_title>Solve linear systems: Ax = b solver (like np.linalg.solve, MATLAB ).</issue_title>
<issue_description>Look to things like

https://github.com/Quafadas/vecxt/blob/main/vecxt/src-jvm/eig.scala

for examples on how to structure this. Use the lapack facade on the JVM, and leave a placeholder on native and JS.

Write comprehensive test coverage and make sure that tests on the JVM pass.

Review the test coverage for completness and mark that out of 10, make sure the score is above 8 and that the tests pass before asking for a review. </issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: Quafadas <24899792+Quafadas@users.noreply.github.com>
Copilot AI changed the title [WIP] Add Ax = b solver using LAPACK on JVM Add linear system solver (Ax = b) using LAPACK dgesv Nov 24, 2025
Copilot AI requested a review from Quafadas November 24, 2025 13:36
Comment thread vecxt/src-jvm/solve.scala
@Quafadas

Copy link
Copy Markdown
Owner

@copilot seems like CI is failing. Can you check?

- Move dimension check (A.rows == b.rows) to use inline bounds check pattern similar to other matrix checks
- Remove redundant require statement inside if bc block
- Keep NaN checks in if bc block following eig.scala pattern
- Update test to expect MatrixDimensionMismatch exception type
- All 25 tests still pass

Co-authored-by: Quafadas <24899792+Quafadas@users.noreply.github.com>

Copilot AI commented Nov 24, 2025

Copy link
Copy Markdown
Contributor Author

@copilot seems like CI is failing. Can you check?

CI is now passing. All platforms (JVM, JS, Native) compile successfully and all 25 tests pass on JVM. The issues have been resolved in commit 2e1bd44.

Comment thread vecxt/src-jvm/solve.scala Outdated
Per code review feedback, the manual element-by-element copy into a new array is unnecessary since the solve(Matrix) method already makes a copy via deepCopy. Now directly returns xMatrix.raw which contains the solution. This improves performance by avoiding an extra O(n) copy operation.

All 25 tests continue to pass.

Co-authored-by: Quafadas <24899792+Quafadas@users.noreply.github.com>
@Quafadas Quafadas marked this pull request as ready for review November 25, 2025 07:31
Copilot AI review requested due to automatic review settings November 25, 2025 07:31
@Quafadas Quafadas merged commit 71c8142 into main Nov 25, 2025
4 checks passed
@Quafadas Quafadas deleted the copilot/add-ax-equals-b-solver branch November 25, 2025 07:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR implements a linear system solver for solving Ax = b using LAPACK's dgesv routine on the JVM, with placeholder implementations for JS and Native platforms. The implementation follows established patterns from existing LAPACK-based operations (eigenvalues, SVD, Cholesky) in the repository.

Key Changes

  • Adds JVM-specific solve implementation using LAPACK dgesv with LU decomposition
  • Provides two API variants: one for matrix right-hand sides and one for vector (Array) right-hand sides
  • Includes comprehensive validation (bounds checking, NaN detection, dimension compatibility)
  • Adds 25 comprehensive tests covering various matrix types and edge cases

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
vecxt/src-jvm/solve.scala JVM implementation using LAPACK dgesv for solving linear systems with proper error handling and validation
vecxt/src-js-native/solve.scala Placeholder implementation for JS and Native platforms (not yet implemented)
vecxt/src/all.scala Exports Solve module to make it available in the public API
vecxt/test/src-jvm/solve.test.scala Comprehensive test suite with 25 tests covering identity matrices, diagonal matrices, triangular matrices, singular matrices, edge cases, and error conditions

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.

Solve linear systems: Ax = b solver (like np.linalg.solve, MATLAB ).

3 participants