headmyshoulder / odeint-v2

odeint - solving ordinary differential equations in c++ v2
http://headmyshoulder.github.com/odeint-v2/
Other
337 stars 102 forks source link

odeint and eigen3.3 #194

Closed boris-il-forte closed 5 years ago

boris-il-forte commented 7 years ago

Hi,

I'm having compilation issues on debian testing with boost 1.61.0.2 and eigen3 3.3~beta2-2.

It seemes that, the header file eigen_algebra.hpp, is using a template named scalar_add_op, that is not present in that version of eigen. I suspect that it was refactored to scalar_sum_op.

i've just changed scalar_add_op to scalar_sum_op in the whole file, and now it works just fine.

Anybody can confirm this issue?

headmyshoulder commented 7 years ago

I haven’t checked completely but it seems that scalar_add_op is not present in eigen 3.3 anymore.

I don’t know how to solve this issue know. There is no notion of third party libraries or their versions in odeint right now. Seems that we need to think about this now.

On Sep 27, 2016, at 7:22 AM, Davide Tateo notifications@github.com wrote:

Hi,

I'm having compilation issues on debian testing with boost 1.61.0.2 and eigen3 3.3~beta2-2.

It seemes that, the header file eigen_algebra.hpp, is using a template named scalar_add_op, that is not present in that version of eigen. I suspect that it was refactored to scalar_sum_op.

i've just changed scalar_add_op to scalar_sum_op in the whole file, and now it works just fine.

Anybody can confirm this issue?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/headmyshoulder/odeint-v2/issues/194, or mute the thread https://github.com/notifications/unsubscribe-auth/AAzSlPiRA7JgDe-wVryGoN_utGJbt0hJks5quSa5gaJpZM4KHuph.

boris-il-forte commented 7 years ago

Maybe, a quick solution for this issue is to use the Eigen macros: EIGEN_WORLD_VERSION, EIGEN_MAJOR_VERSION, EIGEN_MINOR_VERSION

the define conditionally with a ifdef a macro or a typedef to switch from scalar_add_op to scalar_sum_op based on eigen major version

crmoore commented 7 years ago

Renaming scalar_add_op to scalar_sum_op did not work for me. I had to copy the definition of scalar_add_op from the Eigen commit where it was removed and conditionally compile it at the top of the header:

#if EIGEN_VERSION_AT_LEAST(3,3,0)
namespace Eigen {
namespace internal{

template<typename Scalar>
struct scalar_add_op {
  // FIXME default copy constructors seems bugged with std::complex<>
  EIGEN_DEVICE_FUNC inline scalar_add_op(const scalar_add_op& other) : m_other(other.m_other) { }
  EIGEN_DEVICE_FUNC inline scalar_add_op(const Scalar& other) : m_other(other) { }
  EIGEN_DEVICE_FUNC inline Scalar operator() (const Scalar& a) const { return a + m_other; }
  template <typename Packet>
  EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE const Packet packetOp(const Packet& a) const
  { return internal::padd(a, pset1<Packet>(m_other)); }
  const Scalar m_other;
};
template<typename Scalar>
struct functor_traits<scalar_add_op<Scalar> >
{ enum { Cost = NumTraits<Scalar>::AddCost, PacketAccess = packet_traits<Scalar>::HasAdd }; };

} // namespace internal

#endif // EIGEN_VERSION_AT_LEAST(3,3,0)

ref: https://bitbucket.org/eigen/eigen/commits/6ae7c2f275d7517edcaf843b28982506b0a4c4fe

hazelnusse commented 5 years ago

I'd like to help get this fixed. Neither of the solutions above worked for me either. I've created a repo with a minimal example that I'd like to get working which I (or others) can be work from to get a solution: https://github.com/hazelnusse/odeint_eigen_mwe

I was thinking that Boost would probably need to be pulled in from a git repo, but maybe there is a way to do just the boostorg/odeint repo and not the the entire boostorg/boost meta repo with all of the git submodules?

hazelnusse commented 5 years ago

@headmyshoulder @mariomulansky Any thoughts?

mariomulansky commented 5 years ago

@hazelnusse I uploaded a change that should make odeint compatible with both old and new eigen. I tried with Eigen 3.3.7 and all tests compiled and ran successfully. It would be great if you could give it a try, if it works for you as well I will merge it: https://github.com/headmyshoulder/odeint-v2/pull/237

hazelnusse commented 5 years ago

Hi @mariomulansky, I tested that out and it works for me. Would it be worth getting somebody from Eigen to take a look since you are depending on internal behavior of Eigen to make it work? Perhaps they could suggest a way that wouldn't depend on the internal namespace?

hazelnusse commented 5 years ago

For reference, this is the commit that tests out the change you made: https://github.com/hazelnusse/odeint_eigen_mwe/commit/9c5253495e32cf8762afb023940b2f32f33f5ed6

mariomulansky commented 5 years ago

@hazelnusse There is certainly a better, official, way of supporting Eigen types in controlled steppers, but it would mean significantly more work on our end. I think the current workaround is a good compromise, although we might have to go back and fix things again if a major change inside Eigen breaks us, but this should be rare/unlikely.

hazelnusse commented 5 years ago

@mariomulansky Thanks for getting this merged! Will this commit make it's way in to Boost 1.70? Anything I can do to help make that happen?

hazelnusse commented 5 years ago

@mariomulansky @headmyshoulder Looks like this didn't get into Boost 1.70.0. What needs to happen to get it into the next boost? It isn't clear to me how commits in headmyshoulder/odeint-v2/master go to boostorg/odeint and then subsequently into released versions.

mariomulansky commented 5 years ago

@hazelnusse all manual labor :) I just merged the changes into boost master: https://github.com/boostorg/odeint/pull/36 should be part of the next boost release.