Skip to content

Conversation

gojakuch
Copy link
Collaborator

Fixes: #1920

@gojakuch gojakuch added 1:ST:WIP This PR is a work in progress. Not ready for review. is:bugfix This fixes a bug labels Sep 25, 2025
@ginkgo-bot ginkgo-bot added mod:core This is related to the core module. mod:cuda This is related to the CUDA module. type:preconditioner This is related to the preconditioners type:matrix-format This is related to the Matrix formats mod:hip This is related to the HIP module. labels Sep 25, 2025
Comment on lines +1402 to +1409
// check the sorting
bool sorted;
is_sorted_by_column_index(exec, a, &sorted);
if (!sorted)
throw std::invalid_argument("spgeam: a is not sorted by column index");
is_sorted_by_column_index(exec, b, &sorted);
if (!sorted)
throw std::invalid_argument("spgeam: b is not sorted by column index");
Copy link
Member

Choose a reason for hiding this comment

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

should it be in the data validation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

where exactly?

@upsj
Copy link
Member

upsj commented Sep 29, 2025

I am working on a PR that introduces a better interface for SpGEMM/SpGEAM, and would make this change obsolete, I would suggest holding off on it until that PR is ready.

@gojakuch
Copy link
Collaborator Author

@upsj so did your PR make these changes unneeded effectively solving the issue?

@upsj
Copy link
Member

upsj commented Oct 13, 2025

@gojakuch It added a new interface with clear documentation, which is at least an improvement - I think adding a check here could be detrimental, since the cost of an SpGEAM is in the same order of magnitude as the cost of the sortedness check in the success case.

@gojakuch gojakuch closed this Oct 17, 2025
@gojakuch
Copy link
Collaborator Author

I'll open another PR that just adds the sorting of the matrix in the preconditioner to fix the issue using the new interface without adding the sorting check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1:ST:WIP This PR is a work in progress. Not ready for review. is:bugfix This fixes a bug mod:core This is related to the core module. mod:cuda This is related to the CUDA module. mod:hip This is related to the HIP module. type:matrix-format This is related to the Matrix formats type:preconditioner This is related to the preconditioners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Schwarz with L1 can fail if diagonal matrix is not sorted

4 participants