Refactor Analysis module: distance functions

[jngrad]

Description of changes:

• remove min_dist2()
• move logic from dist_to() into system.distance_vec()
• remove unused code
• fix a regression in PidProfileObservable introduced by Reshape output of Observables #3560
• fix a regression in the OpenGL visualizer introduced by Visualizer cleanup #3575

[review-notebook-app]

Check out this pull request on 

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

[jngrad]

@RudolfWeeber I'm splitting the work in multiple PRs. I'm converting the analysis functions rdf, linear_momentum and angular_momentum as observables in another branch. It's gonna be a substantial extension of the Observable framework. The current framework doesn't scale well, e.g. the PidProfileObservable assumes a 3D profile (to get a 1D profile, one has to duplicate the 3D profile class), and has a lot of code duplication with CylindricalProfile for no good reason (from what I understand, it's only to get class attributes named min_r/min_phi instead of min_x/min_y). A solution of the style template <size_t dimension=3> class ProfileObservable leads to compiler errors because the templated classes have the same symbol. The long-term solution here is probably to have a templated class with structs as template argument, e.g. Cartesian1D, Cartesian3D, Cylindrical3D, Radial1D, etc. As an illustration, the RDF requires the creation of either a PtypeProfileObservable1d -> {PtypeObservable, ProfileObservable1d} class or a RadialProfile class.

[codecov]

Codecov Report

Merging #3586 into python will increase coverage by <1%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           python   #3586    +/-   ##
=======================================
+ Coverage      88%     88%   +<1%     
=======================================
  Files         528     528            
  Lines       23607   23590    -17     
=======================================
- Hits        20785   20777     -8     
+ Misses       2822    2813     -9

Impacted Files	Coverage Δ
src/core/statistics.hpp	86% <ø> (-2%)	⬇️
src/core/observables/PidProfileObservable.hpp	100% <100%> (ø)	⬆️
src/core/statistics.cpp	76% <100%> (+1%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ba2d1d...638348c. Read the comment docs.

[KaiSzuttor]

Maybe we can get rid of distto

[KaiSzuttor]

Concerning the histogram observables: this is probably a bit more involved because currently it also has the issue that histogram information cannot be retrieved. Also it may be worth to first refactor the observable framework and afterwards think about interaction with the histograms?

[jngrad]

I haven't looked into histograms, not sure how much of if it will overlap with my work on the ProfileObservable classes. I'm not really convinced by the current framework, for example ProfileObservable inherits from Observable but CylindricalProfile doesn't. It's unclear whether SphericalProfile should derive from Observable or not. Also, the way CylindricalProfile is currently used, it would simplify the code to have it inherit from Observable, but I'm not sure it's the right direction. Having a template class ProfileObservable<coord_system> inheriting from Cartesian/Cylindrical/Spherical would be less confusing.

[RudolfWeeber]

```

@jngrad commented on this pull request.

_____

In src/python/espressomd/system.pyx <#3586 (comment)> : > @@ -359,6 +360,63 @@ cdef class System: return make_array_locked(mi_vec) + def dist_to(self, id=None, pos=None): would you like me to change the system.distance_vec() to take either particles or 3d vectors, so that we can use your one-liner?

Yes. There should be only system.distance and system.distance_vec. Both should take either a 3-array or a ParticleHandle as either of their arguments. ```