This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen

Bug 583

Summary: Householder QR preconditioner asserts with unsigned index type
Product: Eigen Reporter: Tim Besard <tim.besard>
Component: QRAssignee: Nobody <eigen.nobody>
Status: RESOLVED FIXED    
Severity: Unknown CC: chtz, gael.guennebaud
Priority: Normal    
Version: 3.2   
Hardware: All   
OS: All   
Whiteboard:
Bug Depends on:    
Bug Blocks: 387    
Attachments:
Description Flags
Test-case to trigger the assertion.
none
Compilation log.
none
Backtrace from the triggered assertion. none

Description Tim Besard 2013-04-11 09:43:28 UTC
Created attachment 328 [details]
Test-case to trigger the assertion.

If I use a unsigned index type (#define EIGEN_DEFAULT_DENSE_INDEX_TYPE unsigned), all householder QR preconditioners trigger an assertion. Looking at the source it seems that the SVD code doesn't seem to be written with unsigned indexes in mind (for loops while --i > 0, etc).
Comment 1 Tim Besard 2013-04-11 09:44:37 UTC
Created attachment 329 [details]
Compilation log.

This compilation log, produces with Clang -Wall -Wextra, lists some of the code constructs which seem incapable of working with unsigned indices.
Comment 2 Tim Besard 2013-04-11 09:44:59 UTC
Created attachment 330 [details]
Backtrace from the triggered assertion.
Comment 3 Christoph Hertzberg 2013-04-18 14:32:49 UTC
Index is generally supposed to be a signed integer type (maybe this should be documented where EIGEN_DEFAULT_DENSE_INDEX_TYPE is described).
There may be many parts which also work with unsigned integers, but you can't rely on this always being the case.
Do you have a use case where you need that extra bit for the index type (and where going to the next bigger int-type is not an option)?
Comment 4 Tim Besard 2013-04-18 17:55:07 UTC
No, I just tried to use it since I prefer unsigned types for values which shouldn't go negative (such as matrix dimensions). Certainly no must-have, although it shouldn't fail as horribly as it did.
Comment 5 Christoph Hertzberg 2013-04-18 19:21:37 UTC
I guess changing that design decision is hardly possible now. However, it should be possible to add a (more descriptive) static assertion if the user provided index type is not signed.
Comment 6 Gael Guennebaud 2013-06-10 17:17:02 UTC
https://bitbucket.org/eigen/eigen/commits/a8c411cca7a3/
Changeset:   a8c411cca7a3
User:        ggael
Date:        2013-06-10 17:16:16
Summary:     Fix bug 583: add compile-time check that DenseIndex is signed
Comment 7 Nobody 2019-12-04 12:14:51 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/583.