Re: [eigen] DenseCoeffsBase

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

Thanks a lot for the test case, it's fixed now and I've incorporated
your test case to our unit tests.

Benoit

2010/5/9 Trevor Irons <trevorirons@xxxxxxxxx>:
> This exhibits the problem.
>
> #include "Eigen/Eigen"
> #include <complex>
>
> typedef double Real;
>
> int main() {
>     Eigen::Matrix<std::complex<Real>, 3, 1>  test;
>     Eigen::Matrix<Real, 3, 1>                rv;
>     rv = test.imag();
>     // rv = test.real(); // this compiles fine.
> }
>
>
>
>   O__  ---- Trevor Irons, PhD Candidate
>  c/ /'_ --- Dept. of Geophysics
> (*) \(*) -- Colorado School of Mines
> ~~~~~~~~~~~  Ph:  (+01) 720.635.8218
> ~~~~~~~~~~~  (tirons@xxxxxxxxx)
>
>
> On 9 May 2010 09:14, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>>
>> yes, it's the most recent indeed.
>>
>> in all likeliness you found a real problem and it's our unit-tests
>> that were insufficient to reveal them. i'll be very interested in your
>> test case when you can send it.
>>
>> Benoit
>>
>> 2010/5/9 Trevor Irons <trevorirons@xxxxxxxxx>:
>> > I'm pretty sure this is the most recent.
>> >
>> > [tirons@myhost eigen]$ hg pull -u
>> > pulling from https://bitbucket.org/eigen/eigen/
>> > searching for changes
>> > no changes found
>> >
>> > [tirons@myhost eigen]$ hg parent
>> > changeset:   2810:e026fade0a98
>> > tag:         tip
>> > user:        Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
>> > date:        Sun May 09 09:41:54 2010 -0400
>> > summary:     add important comment and move stride helpers to
>> > DenseCoeffsBase.h
>> >
>> > [tirons@myhost eigen]$ gcc --version
>> > gcc (GCC) 4.5.0
>> > Copyright (C) 2010 Free Software Foundation, Inc.
>> > This is free software; see the source for copying conditions.  There is
>> > NO
>> > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
>> > PURPOSE.
>> >
>> > I'll get a minimal case put together, probably not until tomorrow. Maybe
>> > I'll find i'm doing something silly in the process.
>> >
>> > No worries, nothing critical right now.
>> >
>> > Thanks,
>> >
>> > Trevor
>> >
>> >   O__  ---- Trevor Irons, PhD Candidate
>> >  c/ /'_ --- Dept. of Geophysics
>> > (*) \(*) -- Colorado School of Mines
>> > ~~~~~~~~~~~  Ph:  (+01) 720.635.8218
>> > ~~~~~~~~~~~  (tirons@xxxxxxxxx)
>> >
>> >
>> > On 9 May 2010 09:04, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>> >>
>> >> Hi,
>> >>
>> >> are you sure that you have the latest eigen revision? I had this error
>> >> half-way through my changes, but thought I had fixed it.
>> >>
>> >> do:
>> >>
>> >> hg parent
>> >>
>> >> Otherwise, yes a minimal example would be useful, as well as knowing
>> >> your gcc version.
>> >>
>> >> Benoit
>> >>
>> >> 2010/5/9 Trevor Irons <trevorirons@xxxxxxxxx>:
>> >> > FYI
>> >> >
>> >> > I'm getting a compile error in some code that looks related to this
>> >> > that
>> >> > popped up after this latest commit. Maybe just a complex piece is
>> >> > missing?
>> >> > ...maybe I'm doing something wrong.
>> >> >
>> >> > I can piece together a minimal example if needed, but maybe the
>> >> > problem
>> >> > is
>> >> > obvious to you from the error?
>> >> >
>> >> > Thanks,
>> >> > Trevor
>> >> >
>> >> > g++ -o src/receiverpoints.os -c -O3 -Wall -Wno-long-long -DNDEBUG
>> >> > -Werror
>> >> > -Wfatal-errors -msse -fPIC -DLEMMAUSEVTK -Wno-deprecated -fopenmp
>> >> > -DLEMMAUSEOMP -I/home/tirons/src/eigen -Iinclude
>> >> > -I/usr/include/vtk-5.4
>> >> > src/receiverpoints.cpp
>> >> > In file included from /home/tirons/src/eigen/Eigen/Core:257:0,
>> >> >                  from include/lemma..h:110,
>> >> >                  from include/referencecountedobject.h:37,
>> >> >                  from include/receiverpoints.h:50,
>> >> >                  from src/receiverpoints.cpp:34:
>> >> > /home/tirons/src/eigen/Eigen/src/Core/CwiseUnaryView.h: In member
>> >> > function
>> >> > 'Eigen::CwiseUnaryViewImpl<ViewOp, MatrixType,
>> >> > Eigen::Dense>::CoeffReturnType Eigen::CwiseUnaryViewImpl<ViewOp,
>> >> > MatrixType,
>> >> > Eigen::Dense>::coeff(int) const [with ViewOp =
>> >> > Eigen::ei_scalar_imag_ref_op<std::complex<double> >, MatrixType =
>> >> > Eigen::Matrix<std::complex<double>, 3, 1>,
>> >> > Eigen::CwiseUnaryViewImpl<ViewOp,
>> >> > MatrixType, Eigen::Dense>::CoeffReturnType = const double&]':
>> >> > /home/tirons/src/eigen/Eigen/src/Core/CwiseUnaryOp.h:124:75:
>> >> > instantiated
>> >> > from 'Eigen::CwiseUnaryOpImpl<UnaryOp, XprType, Eigen::Dense>::Scalar
>> >> > Eigen::CwiseUnaryOpImpl<UnaryOp, XprType, Eigen::Dense>::coeff(int)
>> >> > const
>> >> > [with UnaryOp = Eigen::ei_scalar_multiple_op<double>, XprType =
>> >> >
>> >> > Eigen::CwiseUnaryView<Eigen::ei_scalar_imag_ref_op<std::complex<double>
>> >> > >,
>> >> > Eigen::Matrix<std::complex<double>, 3, 1> >,
>> >> > Eigen::CwiseUnaryOpImpl<UnaryOp, XprType, Eigen::Dense>::Scalar =
>> >> > double]'
>> >> > /home/tirons/src/eigen/Eigen/src/Core/DenseCoeffsBase.h:450:7:
>> >> > instantiated from 'void Eigen::DenseCoeffsBase<Derived,
>> >> > true>::copyCoeff(int, const Eigen::DenseBase<OtherDerived>&) [with
>> >> > OtherDerived =
>> >> > Eigen::CwiseUnaryOp<Eigen::ei_scalar_multiple_op<double>,
>> >> >
>> >> > Eigen::CwiseUnaryView<Eigen::ei_scalar_imag_ref_op<std::complex<double>
>> >> > >,
>> >> > Eigen::Matrix<std::complex<double>, 3, 1> > >, Derived =
>> >> > Eigen::Matrix<double, 3, 1>]'
>> >> > /home/tirons/src/eigen/Eigen/src/Core/Assign.h:186:5:   instantiated
>> >> > from
>> >> > 'static void
>> >> > Eigen::ei_assign_LinearTraversal_CompleteUnrolling<Derived1,
>> >> > Derived2, Index, Stop>::run(Derived1&, const Derived2&) [with
>> >> > Derived1 =
>> >> > Eigen::Matrix<double, 3, 1>, Derived2 =
>> >> > Eigen::CwiseUnaryOp<Eigen::ei_scalar_multiple_op<double>,
>> >> >
>> >> > Eigen::CwiseUnaryView<Eigen::ei_scalar_imag_ref_op<std::complex<double>
>> >> > >,
>> >> > Eigen::Matrix<std::complex<double>, 3, 1> > >, int Index = 0, int
>> >> > Stop =
>> >> > 3]'
>> >> > /home/tirons/src/eigen/Eigen/src/Core/Assign.h:309:5:   instantiated
>> >> > from
>> >> > 'static void Eigen::ei_assign_impl<Derived1, Derived2, 1,
>> >> > 2>::run(Derived1&,
>> >> > const Derived2&) [with Derived1 = Eigen::Matrix<double, 3, 1>,
>> >> > Derived2
>> >> > =
>> >> > Eigen::CwiseUnaryOp<Eigen::ei_scalar_multiple_op<double>,
>> >> >
>> >> > Eigen::CwiseUnaryView<Eigen::ei_scalar_imag_ref_op<std::complex<double>
>> >> > >,
>> >> > Eigen::Matrix<std::complex<double>, 3, 1> > >]'
>> >> > /home/tirons/src/eigen/Eigen/src/Core/Assign.h:472:3:   instantiated
>> >> > from
>> >> > 'Derived& Eigen::DenseBase<Derived>::lazyAssign(const
>> >> > Eigen::DenseBase<OtherDerived>&) [with OtherDerived =
>> >> > Eigen::CwiseUnaryOp<Eigen::ei_scalar_multiple_op<double>,
>> >> >
>> >> > Eigen::CwiseUnaryView<Eigen::ei_scalar_imag_ref_op<std::complex<double>
>> >> > >,
>> >> > Eigen::Matrix<std::complex<double>, 3, 1> > >, Derived =
>> >> > Eigen::Matrix<double, 3, 1>]'
>> >> > /home/tirons/src/eigen/Eigen/src/Core/DenseStorageBase.h:306:46:
>> >> > instantiated from 'Derived&
>> >> > Eigen::DenseStorageBase<Derived>::lazyAssign(const
>> >> > Eigen::DenseBase<OtherDerived>&) [with OtherDerived =
>> >> > Eigen::CwiseUnaryOp<Eigen::ei_scalar_multiple_op<double>,
>> >> >
>> >> > Eigen::CwiseUnaryView<Eigen::ei_scalar_imag_ref_op<std::complex<double>
>> >> > >,
>> >> > Eigen::Matrix<std::complex<double>, 3, 1> > >, Derived =
>> >> > Eigen::Matrix<double, 3, 1>]'
>> >> > /home/tirons/src/eigen/Eigen/src/Core/Assign.h:492:123:
>> >> > instantiated
>> >> > from
>> >> > 'static Derived& Eigen::ei_assign_selector<Derived, OtherDerived,
>> >> > false,
>> >> > false>::run(Derived&, const OtherDerived&) [with Derived =
>> >> > Eigen::Matrix<double, 3, 1>, OtherDerived =
>> >> > Eigen::CwiseUnaryOp<Eigen::ei_scalar_multiple_op<double>,
>> >> >
>> >> > Eigen::CwiseUnaryView<Eigen::ei_scalar_imag_ref_op<std::complex<double>
>> >> > >,
>> >> > Eigen::Matrix<std::complex<double>, 3, 1> > >]'
>> >> > /home/tirons/src/eigen/Eigen/src/Core/DenseStorageBase.h:473:98:
>> >> > instantiated from 'Derived&
>> >> > Eigen::DenseStorageBase<Derived>::_set_noalias(const
>> >> > Eigen::DenseBase<OtherDerived>&) [with OtherDerived =
>> >> > Eigen::CwiseUnaryOp<Eigen::ei_scalar_multiple_op<double>,
>> >> >
>> >> > Eigen::CwiseUnaryView<Eigen::ei_scalar_imag_ref_op<std::complex<double>
>> >> > >,
>> >> > Eigen::Matrix<std::complex<double>, 3, 1> > >, Derived =
>> >> > Eigen::Matrix<double, 3, 1>]'
>> >> > /home/tirons/src/eigen/Eigen/src/Core/DenseStorageBase.h:460:95:
>> >> > instantiated from 'void
>> >> > Eigen::DenseStorageBase<Derived>::_set_selector(const OtherDerived&,
>> >> > const
>> >> > Eigen::ei_meta_false&) [with OtherDerived =
>> >> > Eigen::CwiseUnaryOp<Eigen::ei_scalar_multiple_op<double>,
>> >> >
>> >> > Eigen::CwiseUnaryView<Eigen::ei_scalar_imag_ref_op<std::complex<double>
>> >> > >,
>> >> > Eigen::Matrix<std::complex<double>, 3, 1> > >, Derived =
>> >> > Eigen::Matrix<double, 3, 1>]'
>> >> > /home/tirons/src/eigen/Eigen/src/Core/DenseStorageBase.h:452:7:
>> >> > instantiated from 'Derived&
>> >> > Eigen::DenseStorageBase<Derived>::_set(const
>> >> > Eigen::DenseBase<OtherDerived>&) [with OtherDerived =
>> >> > Eigen::CwiseUnaryOp<Eigen::ei_scalar_multiple_op<double>,
>> >> >
>> >> > Eigen::CwiseUnaryView<Eigen::ei_scalar_imag_ref_op<std::complex<double>
>> >> > >,
>> >> > Eigen::Matrix<std::complex<double>, 3, 1> > >, Derived =
>> >> > Eigen::Matrix<double, 3, 1>]'
>> >> > /home/tirons/src/eigen/Eigen/src/Core/Matrix.h:180:30:   instantiated
>> >> > from
>> >> > 'Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>&
>> >> > Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows,
>> >> > _MaxCols>::operator=(const Eigen::MatrixBase<OtherDerived>&) [with
>> >> > OtherDerived =
>> >> > Eigen::CwiseUnaryOp<Eigen::ei_scalar_multiple_op<double>,
>> >> >
>> >> > Eigen::CwiseUnaryView<Eigen::ei_scalar_imag_ref_op<std::complex<double>
>> >> > >,
>> >> > Eigen::Matrix<std::complex<double>, 3, 1> > >, _Scalar = double, int
>> >> > _Rows =
>> >> > 3, int _Cols = 1, int _Options = 0, int _MaxRows = 3, int _MaxCols =
>> >> > 1,
>> >> > Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols> =
>> >> > Eigen::Matrix<double, 3, 1>]'
>> >> > src/receiverpoints.cpp:204:46:   instantiated from here
>> >> > /home/tirons/src/eigen/Eigen/src/Core/CwiseUnaryView.h:126:75: error:
>> >> > no
>> >> > match for call to '(const
>> >> > Eigen::ei_scalar_imag_ref_op<std::complex<double>
>> >> >>) (const Eigen::DenseStorageBase<Eigen::Matrix<std::complex<double>,
>> >> >> 3,
>> >> >> 1>
>> >> >>::Scalar&)'
>> >> > compilation terminated due to -Wfatal-errors.
>> >> >
>> >> >
>> >> >
>> >> > On 8 May 2010 14:08, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>> >> >>
>> >> >> Hi,
>> >> >>
>> >> >> This is to explain the group of commits that I just made in the dev
>> >> >> branch.
>> >> >>
>> >> >> maybe you remember, not long ago, i was playing with the idea of
>> >> >> adding a DenseDirectAccessBase class, then gave up... well the idea
>> >> >> is
>> >> >> back and this time I believe it's correctly implemented. See class
>> >> >> DenseCoeffsBase.
>> >> >>
>> >> >> Here's a quick rationalization:
>> >> >>
>> >> >> We used to have a lot of methods in DenseBase that only made sense
>> >> >> in
>> >> >> the DirectAccess case. Like coeffRef(), writePacket(), copyCoeff(),
>> >> >> all the strides API...
>> >> >>
>> >> >> That was bad of course (not to mention dangerous as they were trying
>> >> >> to call a method with same name in the derived class).
>> >> >>
>> >> >> The "obvious" fix was to use enable_if on these methods, however
>> >> >>  - that would have been fiddling with their prototypes: better avoid
>> >> >> that if we can
>> >> >>  - a lot of methods were concerned, not just 1 or 2 methods.
>> >> >>
>> >> >> The alternative of using a static assert was not very good here,
>> >> >> because e.g. operator() was overloaded with one variant using direct
>> >> >> access and not the other, so we'd be having issues with overload
>> >> >> resolution.
>> >> >>
>> >> >> So instead, the idea is to introduce a new base class
>> >> >> DenseCoeffsBase
>> >> >> in the hierarchy, implementing a "group enable_if". The methods
>> >> >> prototypes stay the same, they just move to the new base class.
>> >> >>
>> >> >> Tell me if you have an objection against that.
>> >> >>
>> >> >> Benoit
>> >> >>
>> >> >>
>> >> >
>> >> >
>> >>
>> >>
>> >
>> >
>>
>>
>
>