Avoid calling *trsv and *gemv with invalid parameter values#174
Open
rgommers wants to merge 4 commits intoxiaoyeli:masterfrom
Open
Avoid calling *trsv and *gemv with invalid parameter values#174rgommers wants to merge 4 commits intoxiaoyeli:masterfrom
*trsv and *gemv with invalid parameter values#174rgommers wants to merge 4 commits intoxiaoyeli:masterfrom
Conversation
This upstreams a fix that SciPy has carried for a long time. Note that in `dgstrf.c`, the call to `dsnode_bmod` comes before the call to `dpivotL` where a singularity is detected, so the fix can not be lifted up to that file.
When factorizing very sparse or singular matrices, the leading dimension (nsupr) can be smaller than the segment size (segsze), which results in invalid parameters being passed to `trsv`. Add a guard to abort early with a clear error message instead. Upstreamed from SciPy, which has carried this patch for a long time.
The ILU pivot routines had two singularity checks with commented-out fprintf/exit calls that silently continued. Replace these with ABORT() to fail with a clear error message instead. Upstreamed from SciPy, which has carried this patch for a long time.
…ingular matrix When the matrix is singular (pivmax == 0.0), pivptr may be out of bounds for lsub_ptr (pivptr >= nsupr). Add a guard to use diagind as the pivot row when pivptr is out of range. Upstreamed from SciPy, which has carried this patch for a long time. Also cleans up some code that was ifdef'd out already.
Contributor
|
I like the changes! A potential further improvement would be to extend the message "failed to factorize matrix" by reason given in the commit desciptions. @xiaoyeli What do you think? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This upstreams patches that SciPy has carried for a long time, as discussed in gh-167. There are four separate fixes - this PR has them all, one commit per issue. The commit messages have more detailed explanation.
I also investigated moving the fixes around as suggested in gh-167, but that doesn't quite work as commented on in #167 (comment).