Exporter: split functions for reuse #6546
Conversation
789eada to
8765711
Compare
|
@knelli2 on your request I dropped all but the first 3 commits for now. These just split up the functions so they can be reused. Mind taking a look? |
knelli2
left a comment
There was a problem hiding this comment.
Clang-tidy has some reasonable errors as well
| target_points_dv.get(d).set_data_ref( | ||
| const_cast<double*>(gsl::at(target_points, d).data()), |
There was a problem hiding this comment.
Thow some NOLINTs in there or use make_const_view
There was a problem hiding this comment.
I don't think we have make_const_view for double*, so I added the nolint
| template <typename ResultDataType, size_t Dim> | ||
| void _interpolate_to_points( |
There was a problem hiding this comment.
Why the underscores at the beginning of the function names? I thought our convention with functions in cpp files was just an anonymous namespace
There was a problem hiding this comment.
changed underscore to _impl
| template <size_t Dim> | ||
| void _interpolate_to_point( |
There was a problem hiding this comment.
This function doesn't seem to be used anywhere. Is it necessary?
There was a problem hiding this comment.
Oh it will be used by the interpolator class in the next PR. I can remove it for now
|
Thanks for reviewing! I pushed two fixups |
knelli2
left a comment
There was a problem hiding this comment.
Looks good, you can squash
7704d57 to
f1bea2c
Compare
|
Done 👍 |
Proposed changes
Split up the interpolation functions so that they can be reused by the new
PointwiseInterpolatorclass, which holds volume data in memory and supports repeated interpolation to arbitrary points. This can be used to load data once and then interpolate many times, e.g. for ray tracing. A follow-up PR will do the time interpolation that's needed for ray tracing, which will use thePointwiseInterpolatorclass as well.Upgrade instructions
Code review checklist
make docto generate the documentation locally intoBUILD_DIR/docs/html.Then open
index.html.code review guide.
bugfixornew featureif appropriate.Further comments