New user self-registration is disabled due to spam. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Before reporting a bug, please make sure that your Eigen version is up-to-date!
Bug 1061 - Matrix operator== does not check for correct size and returns true for empty matrix comparisons.
Summary: Matrix operator== does not check for correct size and returns true for empty ...
Status: DECISIONNEEDED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - general (show other bugs)
Version: 3.2
Hardware: All All
: Normal Unknown
Assignee: Nobody
URL:
Whiteboard:
Keywords: Documentation
Depends on:
Blocks:
 
Reported: 2015-08-30 00:04 UTC by Simon Lynen
Modified: 2015-09-01 14:40 UTC (History)
3 users (show)



Attachments

Description Simon Lynen 2015-08-30 00:04:35 UTC
Operator == does not check for size and incorrectly returns true for comparison of (partically) dynamic matrix with random matrix.

TEST(A, B) {
  typedef Eigen::Matrix<double, 10, Eigen::Dynamic> Type;
  Type a;
  a.setRandom(10, 5);
  Type b;
  EXPECT_FALSE(a == b);  // Fails.
  Type c;
  c.setRandom(10, 5);
  EXPECT_FALSE(a == c);  // Passes.
}

TEST(A, C) {
  typedef Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic> Type;
  Type a;
  a.setRandom(10, 5);
  Type b;
  EXPECT_FALSE(a == b);  // Fails.
  Type c;
  c.setRandom(10, 5);
  EXPECT_FALSE(a == c);  // Passes.
}
Comment 1 Christoph Hertzberg 2015-08-31 11:53:04 UTC
So far, we had a==b be equivalent to a.cwiseEqual(b).all(), which means we assume (and assert) that the sizes match. But checking for matching sizes before that actually makes sense (and I doubt it would break anything). Should we want to keep the current behavior, we must at least document it.
If we change it, we should suggest the alternative .cwiseEqual().all(), in case someone really wishes to avoid the minor overhead of checking the dimension first.
Comment 2 Gael Guennebaud 2015-09-01 14:40:33 UTC
Such a change would make operator== equivalent to Matlab's isequal behavior, so why not.

Another solution would be to add a isEqual() member (and maybe even a isEqualN() to deal with NaN) mimicking Matlab's isequal*, but if we do so, then for consistency we should probably also update isApprox to check for the size first.

Note You need to log in before you can comment on or make changes to this bug.