Skip to content

Conversation

@aartbik
Copy link
Collaborator

@aartbik aartbik commented Jul 25, 2025

(1) allow a mix of memory spaces inside the sparse tensor;
    ultimately, only the visibility matters, not the exact consistency

(2) remove the runtime check on offset = {-1,0,+1} for now;
    I see no other solution than copying this to host or having
    some kernel check on device, which all seems too costly;
    given the very limited usage of UST, we just assume the solve
    is only called on tridiag with these offsets

    (1) allow a mix of memory spaces inside the sparse tensor;
        ultimately, only the visibility matters, not the exact consistency

    (2) remove the runtime check on offset = {-1,0,+1} for now;
        I see no other solution than copying this to host or having
        some kernel check on device, which all seems too costly;
        given the very limited usage of UST, we just assume the solve
        is only called on tridiag with these offsets
@copy-pr-bot
Copy link

copy-pr-bot bot commented Jul 25, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@aartbik aartbik requested review from cliffburdick and Copilot July 25, 2025 19:49
Copy link
Contributor

Copilot AI left a comment

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 removes runtime validation checks for memory spaces and diagonal offsets in sparse tensor operations to improve performance and allow more flexible memory space configurations.

  • Removes memory space consistency checks between sparse tensor components (values, coordinates, positions)
  • Replaces specific diagonal offset validation with a simpler count-only check
  • Adds documentation comments explaining the tridiagonal assumption

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
include/matx/transforms/solve/solve_cusparse.h Simplifies diagonal offset validation by removing specific value checks and keeping only count validation
include/matx/operators/solve.h Adds documentation comment about tridiagonal storage assumption
include/matx/core/sparse_tensor.h Removes memory space consistency checks between sparse tensor components

@cliffburdick
Copy link
Collaborator

/build

@aartbik
Copy link
Collaborator Author

aartbik commented Jul 26, 2025

Unrelated failure?

matxException (matxOutOfMemory: ) - /home/jenkins/workspace/unit-tests/include/matx/core/allocator.h:

@cliffburdick
Copy link
Collaborator

/build

1 similar comment
@cliffburdick
Copy link
Collaborator

/build

@cliffburdick cliffburdick merged commit 4984a57 into NVIDIA:main Jul 28, 2025
1 check passed
@cliffburdick cliffburdick deleted the bik branch July 28, 2025 21:00
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.

2 participants