Details
-
Type:
Improvement
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: afw, ip_diffim, meas_astrom, meas_base, meas_modelfit, shapelet
-
Labels:
-
Story Points:1
-
Epic Link:
-
Sprint:AP S18-6
-
Team:Alert Production
Description
ndarray::EigenView relies on undocumented internals of Eigen. Stop using it, in order to allow upgrading Eigen.
The full conversion consists of two parts:
1) Stop using ndarray::EigenView explicitly in C++ code. That is what this ticket is about.
2) Stop using ndarray::EigenView indirectly via ndarray::Array::asEigen by having that function return an Eigen::Map. That is DM-14728
Attachments
Issue Links
Activity
| Field | Original Value | New Value |
|---|---|---|
| Epic Link |
|
| Risk Score | 0 |
| Summary | Eliminate usage of ndarray::EigenView | Eliminate use of ndarray::EigenView in C++ code |
| Description | ndarray::EigenView relies on undocumented internals of Eigen. Stop using it, in order to allow upgrading Eigen. |
ndarray::EigenView relies on undocumented internals of Eigen. Stop using it, in order to allow upgrading Eigen.
The full conversion consists of two parts: 1) Stop using {{ndarray::EigenView}} explicitly in C++ code. That is what this ticket is about. 2) Stop using {{ndarray::EigenView}} indirectly via {{ndarray::Array::asEigen}} by having that function return an {{Eigen::Map}}. I'll file a separate ticket for that. It will be tricky because we have to be sure memory is being managed properly for all the objects returned by {{asEigen}}. |
| Summary | Eliminate use of ndarray::EigenView in C++ code | Eliminate explicit use of ndarray::EigenView in C++ code |
| Story Points | 10 | 2 |
| Story Points | 2 | 1 |
| Sprint | AP S18-6 [ 686 ] |
| Description |
ndarray::EigenView relies on undocumented internals of Eigen. Stop using it, in order to allow upgrading Eigen.
The full conversion consists of two parts: 1) Stop using {{ndarray::EigenView}} explicitly in C++ code. That is what this ticket is about. 2) Stop using {{ndarray::EigenView}} indirectly via {{ndarray::Array::asEigen}} by having that function return an {{Eigen::Map}}. I'll file a separate ticket for that. It will be tricky because we have to be sure memory is being managed properly for all the objects returned by {{asEigen}}. |
ndarray::EigenView relies on undocumented internals of Eigen. Stop using it, in order to allow upgrading Eigen.
The full conversion consists of two parts: 1) Stop using {{ndarray::EigenView}} explicitly in C++ code. That is what this ticket is about. 2) Stop using {{ndarray::EigenView}} indirectly via {{ndarray::Array::asEigen}} by having that function return an {{Eigen::Map}}. That is |
| Reviewers | Krzysztof Findeisen [ krzys ] | |
| Status | To Do [ 10001 ] | In Review [ 10004 ] |
When we stop using EigenView in pybind11 wrappers then yes, that text will have to be changed (along with all the pybind11 wrappers that use the old technique). However, that work will be done on a different ticket. Initially I am focused on converting our C++ code (excluding pybind11 wrappers) away from ndarray::EigenView. Once that is done we can switch our pybind11 wrappers to use pybind11's built-in Eigen support, which will eliminate the last usage of ndarray::EigenView and (once ndarray is updated to remove EigenView) allow upgrading Eigen.
I suggest a slightly different implementation in meas_modelfit. Otherwise looks good.
Krzysztof Findeisen
added a comment - I suggest a slightly different implementation in meas_modelfit . Otherwise looks good. Krzysztof Findeisen
made changes -
| Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
| Resolution | Done [ 10000 ] | |
| Status | Reviewed [ 10101 ] | Done [ 10002 ] |
In standup you mentioned something about pybind11 conversion being done by Eigen vs. by ndarray. Does the text at https://developer.lsst.io/pybind11/how-to.html#ndarray need to be changed?