Skip to content

Added methods to diagonally scale csr matrices on the left and right.#281

Closed
shakedregev wants to merge 8 commits intodevelopfrom
shaked/diagonal_scaling
Closed

Added methods to diagonally scale csr matrices on the left and right.#281
shakedregev wants to merge 8 commits intodevelopfrom
shaked/diagonal_scaling

Conversation

@shakedregev
Copy link
Copy Markdown
Collaborator

@shakedregev shakedregev commented May 23, 2025

Description

This is necessary to solve the issue of L and U being in the right format. See details here. Additionally, diagonal scaling is needed for SUNDIALS and HyKKT integration.

1 of the 2 steps necessary to close issue 251

Mentions @(user)

Proposed changes

I added methods to diagonally scale CSR matrices on the left and right. This will allow the use of external codes and moving the unit diagonal when we transpose and switch L and U by interpreting their CSC representation as CSR. New unit tests pass.

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask. We're here
to help! This is simply a reminder of what we are going to look for before
merging your code.

  • All tests pass. Code tested on
    • CPU backend
    • CUDA backend
    • HIP backend
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows Re::Solve style guidelines.
  • There are unit tests for the new code.
  • The new code is documented.
  • The feature branch is rebased with respect to the target branch.

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining
why you chose the solution you did and what alternatives you considered, etc.

@shakedregev shakedregev force-pushed the shaked/diagonal_scaling branch from 877bb7c to 694a59a Compare May 23, 2025 14:59
@shakedregev shakedregev requested a review from pelesh May 23, 2025 15:00
@shakedregev shakedregev marked this pull request as draft May 23, 2025 15:01
@shakedregev shakedregev marked this pull request as ready for review May 23, 2025 15:05
@shakedregev shakedregev self-assigned this May 23, 2025
@shakedregev
Copy link
Copy Markdown
Collaborator Author

A clarification: The different order of arguments for leftDiagonalScale(vector_type diag, matrix::Csr A) and rightDiagonalScale(matrix::Csr A, vector_type diag) is intentional to reflect the mathematics. There is no danger of the code compiling if the order of the arguments is switched. The functions have a different stamp so that the user is intentional about which one to call.

@shakedregev
Copy link
Copy Markdown
Collaborator Author

shakedregev commented May 23, 2025

Closing this as it is broken. Replaced by #283.

@shakedregev shakedregev deleted the shaked/diagonal_scaling branch October 9, 2025 20:51
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.

LinSolverDirectKLU quietly assumes each matrix is a CSC matrix

1 participant