DM-14725: Eliminate explicit use of ndarray::EigenView in C++ code #58
Conversation
| ndarray::EigenView<Scalar,1,1,Eigen::ArrayXpr> _data; | ||
| ndarray::EigenView<Scalar,1,1,Eigen::ArrayXpr> _arg; | ||
| ndarray::Array<Scalar,1,1> _data; // ArrayXpr | ||
| ndarray::Array<Scalar,1,1> _arg; // ArrayXpr |
There was a problem hiding this comment.
I don't understand these comments.
There was a problem hiding this comment.
I think they were placeholders. I'll remove them on DM-14625 (since I jumped the gun and merged this branch before I realized you had any suggested changes).
| } | ||
|
|
||
| virtual bool differentiateResiduals( | ||
| ndarray::Array<Scalar const,1,1> const & parameters, | ||
| ndarray::Array<Scalar,2,-2> const & derivatives | ||
| ) const { | ||
| ndarray::EigenView<Scalar,2,-2,Eigen::ArrayXpr> d(derivatives); | ||
| auto d = derivatives.asEigen<Eigen::ArrayXpr>(); | ||
| auto argEigen = _arg.asEigen<Eigen::ArrayXpr>(); |
There was a problem hiding this comment.
You end up doing these conversions a lot. Consider using private getters or simply letting the types of _data and _arg break when you switch to Eigen::Map.
There was a problem hiding this comment.
The plan is as follows: add asEigenArray and asEigenMatrix to ndarray::Array; these will both return Eigen::Map. Then eliminate all use of asEigen in our code, which will eliminate all use of ndarray::EigenView. We can then remove EigenView from ndarray ((at least conditionally when ndarray is built against too new a version of Eigen, but as far as LSST is concerned it will go away).
There was a problem hiding this comment.
How does that reduce the code duplication of translating _data and _arg to Eigen::Map every time you want to use them?
| ndarray::EigenView<Pixel const,1,1,Eigen::ArrayXpr> const & variance | ||
| ndarray::Array<Pixel const,1,1> const & model, | ||
| ndarray::Array<Pixel const,1,1> const & data, | ||
| ndarray::Array<Pixel const,1,1> const & variance |
There was a problem hiding this comment.
👍 for separation of interface and implementation.
No description provided.