| Summary: | Aliasing detected while no alias actually occurs | ||
|---|---|---|---|
| Product: | Eigen | Reporter: | taroxd |
| Component: | General | Assignee: | Nobody <eigen.nobody> |
| Status: | DECISIONNEEDED --- | ||
| Severity: | Crash | CC: | chtz, gael.guennebaud, jacob.benoit.1, taroxd |
| Priority: | Normal | ||
| Version: | 3.3 (current stable) | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
Works fine with g++ and clang++ with Eigen-default and Eigen-3.3. I would still generally advice to explicitly transpose/adjoint row vectors before assigning them to column vectors (at many places this happens implicitly, but you may get unexpected behavior in some situations): A.col(j).head(j) = A.row(j).head(j).transpose(); Also, the shorter version of your loop would be: A.triangularView<Eigen::StrictlyUpper>() = A.transpose(); There is no (easy) way to disable only the transpose_aliasing assertions, and I don't think that this would make sense. I am well aware that a shorter version A.triangularView<Eigen::StrictlyUpper>() = A.transpose(); is available. The code I have provided is only a minimal example that triggers this unexpected behavior. I will be glad to explicitly transpose row vector before assigning, if doing so is not hiding a bug about alias detection. With clang/gcc I don't even get aliasing assertions when removing the `.head(j)` I do get the assertion if I let your loop start at j=0, instead of 1 (did you mis-copy your code snippet?). The simplest workaround for this issue would be to disable the assertion, if the size is 0 or 1. We had that discussion at some place -- but distinguishing all valid from invalid aliasing problems is hard (at least to hard for simple run-time assertions), e.g. for square matrices A.row(i) = A.col(i).transpose(); is always fine, since A(i,i) simply gets assigned to itself, whereas for A.row(i) = A.col(j).transpose(); it depends on the order values get copied, if (i!=j). Christoph, don't you think that .noalias() should turn off the check? Since there are false positives, it makes sense to provide a mechanism to shutdown them, and I think .noalias() is the right tool for that. I don't think users put .noalias() everywhere without thinking. (In reply to Christoph Hertzberg from comment #3) > With clang/gcc I don't even get aliasing assertions when removing the > `.head(j)` > > I do get the assertion if I let your loop start at j=0, instead of 1 (did > you mis-copy your code snippet?). > The simplest workaround for this issue would be to disable the assertion, if > the size is 0 or 1. > > We had that discussion at some place -- but distinguishing all valid from > invalid aliasing problems is hard (at least to hard for simple run-time > assertions), e.g. for square matrices > A.row(i) = A.col(i).transpose(); > is always fine, since A(i,i) simply gets assigned to itself, whereas for > A.row(i) = A.col(j).transpose(); > it depends on the order values get copied, if (i!=j). Oops, I did mis-copy my code snippet. The loop starts at j=0 when I am testing. Sorry for that. (In reply to Gael Guennebaud from comment #4) > Christoph, don't you think that .noalias() should turn off the check? The difference to the product-noalias would be that it just deactivates an assertion, vs actually generating different code. From the current API I would rather expect that .noalias() does more effort to check for accidental aliases (if possible with reasonable effort). The case which caused this assertion would actually be an alias-issue for any size other than 0 (even though the alias would be harmless). So I'd prefer just deactivating the check for size==0 (or size<=1). I fixed this false positive: https://bitbucket.org/eigen/eigen/commits/2ea3c449716a/ User: ggael Summary: Bug 1646: disable aliasing detection for empty and 1x1 expression but we still have false positives, at least: A.row(0) = A.col(0).transpose(); It seems to me that our current checks cannot positively detect aliasing in case of vectors anyway. So we could simply disable this check when the destination is empty or a vector. Am I overseeing some corner cases? I guess we can't (with reasonable effort) detect valid cases like these anyway: A.row(i) = A.col(j).transpose(); (Similar for: v.segment(i,m) = v.segment(j,n), but that is a different issue) So I'm ok with deactivating this assertion for vectors. -- 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/1646. |
Here is my environment information although I don't think this bug is related to this: OS: Windows 10 (17763) x64 Compiler: Visual Studio 2017 Eigen: 3.3.7 Take the following code for example (I am aware that SelfAdointView can achieve the same thing; it is just an example): int n = 100; MatrixXd A = MatrixXd::Random(n, n); for (int j = 1; j < n; ++j) { A.col(j).head(j) = A.row(j).head(j); } std::cout << (A - A.adjoint()).norm() << std::endl; Running this code in debug mode, I got the assertion failure: Assertion failed: (!check_transpose_aliasing_run_time_selector <typename Derived::Scalar,blas_traits<Derived>::IsTransposed,OtherDerived> ::run(extract_data(dst), other)) && "aliasing detected during transposition, use transposeInPlace() " "or evaluate the rhs into a temporary using .eval()", file path\to\eigen\src\core\transpose.h, line 378 Apparently A.col(j).head(j) and A.row(j).head(j) does not share the same memory. It makes no sense to evaluate the rhs into a temporary. Also, if the code is compiled in release mode (with NDEBUG defined), the code runs perfectly. I find out that changing this code to A.col(j).head(j).noalias() = A.row(j).head(j); does not help. But A.col(j).head(j) = A.row(j).adjoint().head(j); works fine. I have no idea when the assertion will be triggered. Do I have a way to explicitly disable it? I am very sure that aliasing does not occur.