Adds a volumetric grid field to help with lookup in 3D. #406

arjo129 · 2022-04-08T12:08:25Z

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

🎉 New feature

Closes #

Summary

The following PR adds a datastructure that represents a sparse volumetric grid. It allows for storing data in 3D.

TODO

Write unit tests
docs

Test it

Checklist

Signed all commits for DCO
Added tests
Added example and/or tutorial
Updated documentation (as needed)
Updated migration guide (as needed)
codecheck passed (See contributing)
All tests passed (See test coverage)
While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

The following PR adds a datastructure that represents a sparse volumetric grid. It allows for storing data in 3D. # TODO - [ ] Write unit tests - [ ] docs Signed-off-by: Arjo Chakravarty <[email protected]>

Signed-off-by: Arjo Chakravarty <[email protected]>

codecov · 2022-04-12T03:38:01Z

Codecov Report

Merging #406 (1038807) into main (120806b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 1038807 differs from pull request most recent head ca1fc75. Consider uploading reports for the commit ca1fc75 to get more accurate results

@@           Coverage Diff            @@
##             main     #406    +/-   ##
========================================
  Coverage   99.73%   99.73%            
========================================
  Files          69       72     +3     
  Lines        6353     6474   +121     
========================================
+ Hits         6336     6457   +121     
  Misses         17       17

Impacted Files	Coverage Δ
include/ignition/math/Helpers.hh	`100.00% <100.00%> (ø)`
include/ignition/math/PiecewiseScalarField3.hh	`100.00% <100.00%> (ø)`
include/ignition/math/VolumetricGridLookupField.hh	`100.00% <100.00%> (ø)`
include/ignition/math/detail/AxisIndex.hh	`100.00% <100.00%> (ø)`

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 120806b...ca1fc75. Read the comment docs.

Signed-off-by: Arjo Chakravarty <[email protected]>

hidmic

A humble first pass until @scpeters gets to this.

hidmic · 2022-04-12T19:05:38Z

include/ignition/math/VolumetricGridLookupField.hh

+    inline namespace IGNITION_MATH_VERSION_NAMESPACE {
+      template<typename T>
+      /// \brief Lookup table for a volumetric dataset. This class is used to
+      /// lookup indices for a large dataset thats organized in a grid. This


@arjo129 nit:

Suggested change

/// lookup indices for a large dataset thats organized in a grid. This

/// lookup indices for a large dataset that's organized in a grid. This

hidmic · 2022-04-12T19:06:03Z

include/ignition/math/VolumetricGridLookupField.hh

+      template<typename T>
+      /// \brief Lookup table for a volumetric dataset. This class is used to
+      /// lookup indices for a large dataset thats organized in a grid. This
+      /// class is not meant to be used by non-grid like data sets. The grid


@arjo129 nit:

Suggested change

/// class is not meant to be used by non-grid like data sets. The grid

/// class is not meant to be used with non-grid like datasets. The grid

hidmic · 2022-04-12T19:07:03Z

include/ignition/math/VolumetricGridLookupField.hh

+          // NOTE: This part of the code assumes an exact grid of points.
+          // The grid may be distorted or the stride between different points
+          // may not be the same, but fundamentally the data is structured in
+          // may It keeps track of the axis indices for each point in the grid.


@arjo129 nit: note got caught in between two trains of thoughts :)

hidmic · 2022-04-12T19:08:33Z

include/ignition/math/VolumetricGridLookupField.hh

+          // The grid may be distorted or the stride between different points
+          // may not be the same, but fundamentally the data is structured in
+          // may It keeps track of the axis indices for each point in the grid.
+          for(auto pt : _cloud)


@arjo129 nit: use const references here.

hidmic · 2022-04-12T19:09:42Z

include/ignition/math/VolumetricGridLookupField.hh

+
+          for(std::size_t i = 0; i < _cloud.size(); ++i)
+          {
+            auto pt = _cloud[i];


@arjo129 nit: use a const reference here

hidmic · 2022-04-12T19:14:19Z

include/ignition/math/detail/AxisIndex.hh

+          }
+          else if (fabs(it->first - _value) < _tol)
+          {
+            // Exact match


@arjo129 meta: why introduce a tolerance to check for almost exact matches when we can return all points and defer to the interpolation algorithm to judge?

So if you see how the GetInterpolators works it returns a variable number of points. By doing this this way it makes for cleaner interpolation code as we can separate the kind of interpolation we want to use (none, linear, bilinear, trilinear) just based on the length of the return vector. If we were to go with the other alternative we would spend a lot more time debugging the corner cases of the interpolation.

@arjo129 hmm, why special casing? Isn't linear (resp. bilinear) interpolation just trilinear interpolation for a point that is on an edge (resp. face) of the 3D volumes that make up its grid? I guess I'm missing some numerical nuances here. Anywho, so long as the tolerance can be adjusted I'm OK with this.

See #412 for my original plan.

hidmic · 2022-04-12T19:20:23Z

include/ignition/math/VolumetricGridLookupField.hh

+        /// four corners, if along an edge two points and if coincident with a
+        /// corner one point. If the data is sparse and missing indices then a
+        /// nullopt will be returned.
+        public: std::vector<std::optional<std::size_t>>


@arjo129 meta: why return a sequence of optional values instead of just a plain sequence of values that may just be empty?

hidmic · 2022-04-12T19:20:34Z

include/ignition/math/VolumetricGridLookupField.hh

+        /// corner one point. If the data is sparse and missing indices then a
+        /// nullopt will be returned.
+        public: std::vector<std::optional<std::size_t>>
+          GetInterpolators(Vector3<T> &_pt) const


@arjo129 nit:

Suggested change

GetInterpolators(Vector3<T> &_pt) const

GetInterpolators(const Vector3<T> &_pt) const

hidmic · 2022-04-12T19:27:33Z

include/ignition/math/VolumetricGridLookupField.hh

+            x_indices_by_lat.AddIndexIfNotFound(pt.X());
+            y_indices_by_lon.AddIndexIfNotFound(pt.Y());
+            z_indices_by_depth.AddIndexIfNotFound(pt.Z());
+          }


@arjo129 meta meta: if AxisIndex::AddIndexIfNotFound() returned the corresponding index, neither AxisIndex::GetNumUniqueIndices() nor AxisIndex::GetIndex() would be necessary (and the same goes for the floating point equality comparison that AxisIndex::GetIndex() is forced to do).

hidmic · 2022-04-12T19:28:29Z

include/ignition/math/VolumetricGridLookupField.hh

+            std::size_t x_index = x_indices_by_lat.GetIndex(pt.X()).value();
+            std::size_t y_index = y_indices_by_lon.GetIndex(pt.Y()).value();
+            std::size_t z_index = z_indices_by_depth.GetIndex(pt.Z()).value();
+            index_table[z_index][y_index][x_index] = i;


@arjo129 meta: should we be checking for duplicates?

Signed-off-by: Arjo Chakravarty <[email protected]>

arjo129 · 2022-04-21T15:11:49Z

Superseded by #412

arjo129 added 2 commits April 8, 2022 19:42

Adds a volumetric grid field to help with lookup in 3D.

6d11ecc

The following PR adds a datastructure that represents a sparse volumetric grid. It allows for storing data in 3D. # TODO - [ ] Write unit tests - [ ] docs Signed-off-by: Arjo Chakravarty <[email protected]>

docs

e86b8bd

Signed-off-by: Arjo Chakravarty <[email protected]>

github-actions bot added the 🌱 garden Ignition Garden label Apr 8, 2022

chapulina added the MBARI-LRAUV Sponsored by MBARI-LRAUV project: https://github.com/osrf/lrauv label Apr 8, 2022

arjo129 added 2 commits April 11, 2022 10:34

Move test to TEST file

c923825

Signed-off-by: Arjo Chakravarty <[email protected]>

Add test cases

6c456cf

Signed-off-by: Arjo Chakravarty <[email protected]>

arjo129 mentioned this pull request Apr 12, 2022

Faster lookup osrf/lrauv#190

Closed

arjo129 added 2 commits April 12, 2022 11:04

codecheck

e58d632

Signed-off-by: Arjo Chakravarty <[email protected]>

fix rounding error

c66d414

Signed-off-by: Arjo Chakravarty <[email protected]>

Increase coverage

cc43c1c

Signed-off-by: Arjo Chakravarty <[email protected]>

arjo129 marked this pull request as ready for review April 12, 2022 08:14

arjo129 requested a review from scpeters as a code owner April 12, 2022 08:14

hidmic reviewed Apr 12, 2022

View reviewed changes

arjo129 added 2 commits April 14, 2022 10:19

Address Feedback

8707e2f

Signed-off-by: Arjo Chakravarty <[email protected]>

Rework API to make it easier to use with interpolation.

ca1fc75

Signed-off-by: Arjo Chakravarty <[email protected]>

arjo129 closed this Apr 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a volumetric grid field to help with lookup in 3D. #406

Adds a volumetric grid field to help with lookup in 3D. #406

arjo129 commented Apr 8, 2022 •

edited

Loading

codecov bot commented Apr 12, 2022 •

edited

Loading

hidmic left a comment

hidmic Apr 12, 2022

hidmic Apr 12, 2022

hidmic Apr 12, 2022

hidmic Apr 12, 2022

hidmic Apr 12, 2022

hidmic Apr 12, 2022

arjo129 Apr 14, 2022

hidmic Apr 18, 2022

arjo129 Apr 19, 2022

hidmic Apr 12, 2022

hidmic Apr 12, 2022

hidmic Apr 12, 2022 •

edited

Loading

hidmic Apr 12, 2022

arjo129 commented Apr 21, 2022 •

edited

Loading

	/// lookup indices for a large dataset thats organized in a grid. This
	/// lookup indices for a large dataset that's organized in a grid. This

	/// class is not meant to be used by non-grid like data sets. The grid
	/// class is not meant to be used with non-grid like datasets. The grid

	GetInterpolators(Vector3<T> &_pt) const
	GetInterpolators(const Vector3<T> &_pt) const

Adds a volumetric grid field to help with lookup in 3D. #406

Adds a volumetric grid field to help with lookup in 3D. #406

Conversation

arjo129 commented Apr 8, 2022 • edited Loading

🎉 New feature

Summary

TODO

Test it

Checklist

codecov bot commented Apr 12, 2022 • edited Loading

Codecov Report

hidmic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hidmic Apr 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arjo129 commented Apr 21, 2022 • edited Loading

arjo129 commented Apr 8, 2022 •

edited

Loading

codecov bot commented Apr 12, 2022 •

edited

Loading

hidmic Apr 12, 2022 •

edited

Loading

arjo129 commented Apr 21, 2022 •

edited

Loading